RFR: Use allocation counter instead of timestamp to track region's age
Zhengyu Gu
zgu at redhat.com
Wed Aug 2 18:25:44 UTC 2017
Thanks for the review.
On 08/02/2017 02:00 PM, Aleksey Shipilev wrote:
> On 08/02/2017 07:49 PM, Zhengyu Gu wrote:
>> Use allocation counter to track heap regions' age for generational and LRU GC.
>>
>> Webrev: http://cr.openjdk.java.net/~zgu/shenandoah/alloc_counter/webrev.00/index.html
>
> Pull the recent updates, partial heuristics now has much less fields:
>
> *) Just unary minus here:
>
> 69 return -1 * compare_by_alloc_seq_ascending(a, b);
>
Fixed
> *) This SIZE_FORMAT_W(8) here, because it should match the header?
>
> 83 SIZE_FORMAT_W(20) ", " SIZE_FORMAT_W(20) ", %8u, {",
>
Fixed
> *) Typos, "Global allocation":
>
> 37 // Gobal alloaction counter, increased for each allocation
>
Fixed
> *) Excess whitespace. Also, comment should mention they are also set on allocation paths.
>
> 60 // Seq numbers are used for generational and Least Recently Used heuristics.
> 61 // They are set when a region is discarded
> 62 uint64_t _first_alloc_seq_num;
> 63 uint64_t _last_alloc_seq_num;
>
Fixed
> *) Do we actually care about "- 1" here?
>
> 104 static uint64_t alloc_seq_num() {
> 105 // Last used seq number
> 106 return AllocSeqNum - 1;
> 107 }
I think it is matter. Now, we can precisely determinate each GC
boundary, as chf commented on irc.
>
> *) Just "...seq_num = ++AllocSeqNum"?
> *) I guess it is safer to compare top() and bottom() directly here, instead of calling superclass?
>
> 36 _last_alloc_seq_num = AllocSeqNum;
> 37 ++ AllocSeqNum;
> 38 if (is_empty()) {
>
bottom() is also a super call. I think is_empty() is more readable.
Updated webrev:
http://cr.openjdk.java.net/~zgu/shenandoah/alloc_counter/webrev.01/index.html
I reran hotspot_gc_shenandoah test
Thanks,
-Zhengyu
> Thanks,
> -Aleksey
>
More information about the shenandoah-dev
mailing list