[9] RFR (S): 8177963: Parallel GC fails fast when per-thread task log overflows

Kim Barrett kim.barrett at oracle.com
Tue Apr 4 19:35:04 UTC 2017


> On Apr 4, 2017, at 5:48 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> Hi all,
> 
>   can I have reviews for this small fix that changes a guarantee() in
> parallel gc when some logging buffer overflows into a warning message
> about the overflow and (continued) reuse of the last entry of that
> buffer so that the user then can then rerun with a larger buffer.
> 
> Since it is so late in the release cycle I would like to keep the fix
> simple instead of rewriting the buffer logic, i.e. make it expandable
> etc. 
> 
> This change has been mostly contributed by Ramki from Twitter, as part
> of JDK-7180571; however we just got this bug from release testing that
> is a duplicate, and he's unavailable to do an RFR right now, and I want
> this fixed asap. I intend to put him as author for this change.
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8177963
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8177963/webrev/
> Testing:
> local testing, new test, jprt
> 
> Thanks,
>   Thomas

I'm surprised the type of the "name" argument for add_task_timestamp
is not "const char*".

I dislike the casts of GCTaskTimeStampEntries to uint (from uintx),
introduced so it can be MIN2'd with _time_stamp_index. Could
GCTaskTimeStampEntries be given a type of uint? Certainly the code
doesn't really support values that won't fit in uint, since
_time_stamp_index will overflow.

I think it would be helpful for print_task_time_stamps to include some
indication of overflow in the header it prints.

But related to both the cast (for MIN2) and the printing, do we really
need _time_stamp_index to be the actual number of add_task_timestamp
calls? Could we instead just have add_task_timestamp do nothing (other
than warn the first time) when the buffer is full?



More information about the hotspot-gc-dev mailing list