RFR (M): 8027476: Improve performance of Stringtable unlink, 8027455: Improve symbol table scan times during gc pauses

[Bengt Rutisson]

Hi Thomas,

On 1/13/14 12:32 PM, Thomas Schatzl wrote:
> Hi Bengt,
>
> On Mon, 2014-01-13 at 09:11 +0100, Bengt Rutisson wrote:
>> Hi Thomas,
>>
>> Looks good. Great work!
> Thanks for the review.
>
>> A couple of minor things. I'll leave it up to you if you want to do any
>> changes. I'm fine with the patch as it is.
>>
>>
>> G1TraceStringSymbolTableScrubbing is now experiemental. It looks like it
>> is more common to have Trace flags be diagnostic. Do you mind changing
>> to diagnostic?
> I would soon like to change this to more detailed output like we do for
> update RS in the young GC pause. I thought that keeping stuff
> experimental helps to be able to remove the flag later :)

OK. I see :) Leave it as experimental.

>
>> g1CollectedHeap.cpp
>>
>> 5248         StringTable::unlink(_is_alive, &strings_processed,
>> &strings_removed);
>> 5249         _strings_processed = strings_processed;
>> 5250         _strings_removed = strings_removed;
>>
>> Maybe the single threaded case could do?
>>
>> StringTable::unlink(_is_alive, &_strings_processed, &_strings_removed);
>>
>> Then these local variables could move in inside the
>> G1CollectedHeap::use_parallel_gc_threads() check.
>>
>> 5230     size_t strings_processed = 0;
>> 5231     size_t strings_removed = 0;
>> 5232     size_t symbols_processed = 0;
>> 5233     size_t symbols_removed = 0;
> That unfortunately does not work, because we need jlong type variables
> for the Atomic operations.

Right. Missed that.

>
> However, while looking at Mikael's suggestions I found that the use of
> size_t in the change does not give any advantage (as the main limitation
> is the underlying hashmap that only allows int typed sizes) except for
> too many casts.

Good! :)

>
> I changed this in http://cr.openjdk.java.net/~tschatzl/8027454/webrev.1/
> to use int's for the sizes.

Looks good. Thanks for fixing these things!

Bengt

>
> Thanks,
>    Thomas
>
>