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