Re: [mat-dev] GarbageCleaner phase code

[Jason Koch <jkoch@xxxxxxxxxxx>]

Hi Andrew/Krum,

I have submitted the diff as a set of patches attached to a new ticket.
https://bugs.eclipse.org/bugs/show_bug.cgi?id=570670

I was unable to upload a branch to gerrit as I receive a 403 when
trying to create a new branch.

I would also be happy to break this part into individual tickets and
submit the changes separately, or push to a/multiple branch if that
makes review easier.

Thanks
Jason

On Wed, Jan 13, 2021 at 1:59 AM Tsvetkov, Krum <krum.tsvetkov@xxxxxxx> wrote:
>
> Hi Jason,
>
> I also think this is a very good improvement and encourage you to go on and raise a bug/contribute the changes.
>
> @Andrew, I think the suggestions you made for testing are good. I don't have some to add.
>
> Regards,
> Krum
>
> On 06.01.21, 23:50, "mat-dev-bounces@xxxxxxxxxxx on behalf of Andrew Johnson"  wrote:
>
>     > Hi all,
>     >
>     > Looking for some input on suitability to raise a bug and submit this
>     > change. If you can advise any additional specific tests to run I
>     > will be happy to build them out and execute them..
>     >
>     > Results on a 73GB file over 5 runs on a 48GB 8+8cores machine are
>     > here, in brief it is 2-4x faster for large heaps:
>     > https://gist.github.com/jasonk000/9f31e2446a84b56f11d494c146969e7b
>     >
>     > I understand that my last round of commits required an additional
>     > pass of discussion on memory footprint. I believe the memory
>     > footprint here would be reduced as it merges the maps during
>     > generation (C maps, scaling to C cores) vs previous approach (N
>     > maps, when the processing was split into N chunks and scaled to
>     > object heap). All existing functional tests work ok.
>     >
>     > There are a sequence of five commits presented here:
>     > https://github.com/jasonk000/eclipse-mat/compare/gc-opt
>     >
>     > Thanks
>     > Jason_______________________________________________
>
>     This looks like a useful improvement. I don't have any tests in mind,
>     but do look at the previous changes to GarbageCleaner.java and what
>     defects led up to the change.
>
>     Tests are useful, but with multi-threaded code then reviews of the
>     code are also important as defects will not be as reproducible.
>
>     For example the change to
>     if (ref != null && ref.get() != null)
>     {
>         return ref.get();
>
>     could cause problem as the soft reference could be cleared between
>     the two get() calls.
>
>     Sometimes a test mode - with more threads or large / smaller
>     buffers, or a constrained heap, or a very large heap dump with
>     nearly 2G objects can help find bugs.
>
>     Do other people have comments?
>
>     Andrew Johnson
>
>     Unless stated otherwise above:
>     IBM United Kingdom Limited - Registered in England and Wales with number
>     741598.
>     Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
>
>
>
> _______________________________________________
> mat-dev mailing list
> mat-dev@xxxxxxxxxxx
> To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/mat-dev