RFR (M): 8035406: Improve data structure for Code Cache remembered sets

[Bengt Rutisson]

Hi Thomas,

I think this looks good. I have a couple of minor comments. Feel free to 
ignore them. I'm fine with pushing this as it is.

Thanks,
Bengt


g1CodeCacheRemSet.hpp

static const int num_entries = 32;

It seems like in the GC code we try to distinguish numerical constants 
by either using all uppercase or using some kind of CamelCase. Would you 
mind changing to NUM_ENTIRES?

grep -r "static const int" src/share/vm/gc_implementation/
src/share/vm/gc_implementation//g1/bufferingOopClosure.cpp: static const 
int MaxOrder = 2;
src/share/vm/gc_implementation//g1/g1GCPhaseTimes.cpp:  static const int 
BUFFER_LEN = 1024;
src/share/vm/gc_implementation//g1/g1GCPhaseTimes.cpp:  static const int 
INDENT_CHARS = 3;
src/share/vm/gc_implementation//g1/heapRegionRemSet.hpp:  static const 
int MaxRecorded = 1000000;
src/share/vm/gc_implementation//g1/heapRegionRemSet.hpp:  static const 
int MaxRecordedEvents = 1000;
src/share/vm/gc_implementation//g1/sparsePRT.hpp:    static const int s 
= MAX2<int>(G1RSetSparseRegionEntries & ~(UnrollFactor - 1), UnrollFactor);
src/share/vm/gc_implementation//shared/gcTimer.hpp:  static const int 
PHASE_LEVELS = 5;
src/share/vm/gc_implementation//shared/gcTimer.hpp:  static const int 
INITIAL_CAPACITY = 10;


bool find(nmethod* method)
This method only returns true or false. I would expect a method called 
"find" to return the found object. How about renaming it to "contains"?


size_t size() const volatile { return word_size(); }
What does "volatile"  here mean and why is it needed here?



On 3/5/14 1:40 PM, Thomas Schatzl wrote:
> Hi all,
>
>    while finalizing another change for review based on this I found a
> small bug: the amount of handed out chunks has not been tracked
> correctly.
>
> New webrev:
> http://cr.openjdk.java.net/~tschatzl/8035406/webrev.3/
>
> Diff to the previous webrev:
> http://cr.openjdk.java.net/~tschatzl/8035406/webrev.3.diff/
>
> The only real change is in this hunk:
>
>   void G1CodeRootSet::free_all_chunks(FreeList<G1CodeRootChunk>* list) {
> +  G1CodeRootSet::_num_chunks_handed_out -= list->count();
>     _free_list.prepend(list);
>   }
>
> The remaining 129 additional lines of change contains some new unit
> testing code.
>
> Testing:
> jprt (also running this test case)
>
> Sorry,
>    Thomas
>