RFR (M) 8059461: Refactor IndexSet for better performance (preliminary)

[Vladimir Kozlov]

I am start worrying about this change.  Nested ResourceMark may cause 
problems.

PhaseLive::getfreeset() uses thread local resource area for 
_free_IndexSet and PhaseLive::compute() resets it on exit:

    // Init the sparse arrays for delta-sets.
    ResourceMark rm;              // Nuke temp storage on exit

But the rest of IndexSet objects uses indexSet_arena which is 
'live_arena' in Register_Allocate(). It lives longer then 
PhaseLive::compute() and PhaseLive::compute() may 'nuke' its data 
incorrectly if you use thread local resource area for everything.

May be you should also separate arenas for BitMap used as IndexSet.

I think you need to instrument JVM to get correct data. If you use 
separate ResourceArea (as in original code 'live_arena') you can print 
its size at the end of its use scope.

I would recommend to run with -Xbatch -XX:-TieredCompilation 
-XX:CICompilerCount=1 to get serialized data which can be compared.

Clean up:

Remove commented assert in lrg_union() since it is not applicable anymore.

Fix style in the code you are changing. No spaces in if():
   if( vec->is_empty() ) return;
should be
   if (vec->is_empty()) {
     return;
   }

Thanks,
Vladimir

On 11/11/14 11:20 AM, Aleksey Shipilev wrote:
> Thanks for review, Vladimir!
>
> FTR, here's an updated webrev:
>   http://cr.openjdk.java.net/~shade/8059461/webrev.03/
>
> On 11/06/2014 01:58 AM, Vladimir Kozlov wrote:
>> Aleksey, can you compare average memory consumed by IndexSet before and
>> after?
>
> Any ideas how to estimate this reliably? Current code has some
> instrumentation, but only about usage, not the concrete footprint. NMT
> shows very flaky numbers for Nashorn/Octane:
>
> $ grep "Compiler" before/*
>   Compiler (reserved=165KB, committed=165KB)
>   Compiler (reserved=174KB, committed=174KB)
>   Compiler (reserved=201KB, committed=201KB)
>   Compiler (reserved=189KB, committed=189KB)
>   Compiler (reserved=192KB, committed=192KB)
>
> $ grep "Compiler" after/*
>   Compiler (reserved=162KB, committed=162KB)
>   Compiler (reserved=167KB, committed=167KB)
>   Compiler (reserved=198KB, committed=198KB)
>   Compiler (reserved=240KB, committed=240KB)
>   Compiler (reserved=188KB, committed=188KB)
>
> $ grep "Arena" before/*
>   Arena Chunk (reserved=10952KB, committed=10952KB)
>   Arena Chunk (reserved=11433KB, committed=11433KB)
>   Arena Chunk (reserved=10506KB, committed=10506KB)
>   Arena Chunk (reserved=10825KB, committed=10825KB)
>   Arena Chunk (reserved=7179KB, committed=7179KB)
>
> $ grep "Arena" after/*
>   Arena Chunk (reserved=19971KB, committed=19971KB)
>   Arena Chunk (reserved=20738KB, committed=20738KB)
>   Arena Chunk (reserved=21090KB, committed=21090KB)
>   Arena Chunk (reserved=22304KB, committed=22304KB)
>   Arena Chunk (reserved=14342KB, committed=14342KB)
>
> It *looks* like a memory leak in Arenas, but it might be as well the
> larger peak usage. Or, it might be NMT not telling me the whole story.
>
> I am running Footprint tests from our performance regression suites now
> -- they will show if there is something real.
>
>> Why you need initialize_in_resource_arena()? by default BitMap() uses
>> resource area:
>>
>> BitMap(idx_t size_in_bits, bool in_resource_area = true);
>
> Yes, pruned that.
>
>> Make lrg_union() PhaseConservativeCoalesce class's method.
>
> Yes, did that.
>
> Thanks,
> -Aleksey.
>
>