8370520: GenShen: Track and report on promotion failures #27962
Conversation
|
👋 Welcome back wkemper! A progress list of the required criteria for merging this PR into |
|
@earthling-amzn This change is no longer ready for integration - check the PR body for details. |
openjdk
bot
added
hotspot-gc
|
@earthling-amzn The following labels will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command. |
Webrevs
|
| size_t get_promotion_failed_count() const { return _promotion_failure_count; } | ||
| size_t get_promotion_failed_words() const { return _promotion_failure_words; } |
There was a problem hiding this comment.
Shouldn't these be AtomicAccess::load(...)-s? I don't think you need memory ordering, but if you are doing the updates atomically somewhere, it stands to reason you want to match the loads with atomics as well.
|
|
||
| const size_t gc_id = heap->control_thread()->get_gc_id(); | ||
|
|
||
| AtomicAccess::inc(&_promotion_failure_count); |
There was a problem hiding this comment.
Just noticing that in the next code block, we acquire the heap->lock(). Could we just use that same heap lock to protect adjustments to _promotion_failure_count and _promotion_failure_words and then we would need to use Atomic access operations?
There was a problem hiding this comment.
Should probably be ok to read these variables without lock. We're only logging these results when evacuation is no longer happening. Right?
There was a problem hiding this comment.
Now that we have this new log message, can we get rid of the "promotion failure messages" for individual objects?
There was a problem hiding this comment.
We only take the heap lock there conditionally when we haven't yet "squelched" the log message. We could change this to a log_debug(gc, plab) level message. The message is still useful when trying to understand the history and context for a how a thread became unable to promote. I'm not completely convinced there aren't still cases where a thread should be able to promote but can't for some (unknown) reason.
There was a problem hiding this comment.
If we made this a log_debug(gc, plab) message, we could make the whole block of code conditional on the log level being enabled.
And only do the work for the message if the log level is enabled.
4 participants
Keep track of promotion failures. Report the number of failures and total number of bytes that could not be promoted. These changes were hoisted out of #27632.
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27962/head:pull/27962$ git checkout pull/27962Update a local copy of the PR:
$ git checkout pull/27962$ git pull https://git.openjdk.org/jdk.git pull/27962/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 27962View PR using the GUI difftool:
$ git pr show -t 27962Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27962.diff
Using Webrev
Link to Webrev Comment