RFR (S): 8177963: Parallel GC fails fast when per-thread task log overflows
thomas.schatzl at oracle.com
Wed Apr 5 11:26:28 UTC 2017
thanks for your review.
On Tue, 2017-04-04 at 15:35 -0400, Kim Barrett wrote:
> > On Apr 4, 2017, at 5:48 AM, Thomas Schatzl <thomas.schatzl at oracle.c
> > om> 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.
Done. Initially I thought of that as well, but didn't because I wanted
to keep the changes minimal. Since you are also pointing this out...
> 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?
Continuing to count gives an indication on how large the user needs to
size this buffer without too many retries.
If you think this is a problem (because of overflow after 4M tasks), I
can change that again.
test case, jprt
More information about the hotspot-gc-dev