8258255: Move PtrQueue active flag to SATBMarkQueue #1820
Conversation
|
👋 Welcome back kbarrett! A progress list of the required criteria for merging this PR into |
|
@kimbarrett The following label 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 list. If you would like to change these labels, use the /label pull request command. |
Webrevs
|
| @@ -95,7 +103,9 @@ class SATBMarkQueueSet: public PtrQueueSet { | |||
| // These are rarely (if ever) changed, so same cache line as count. | |||
| size_t _process_completed_buffers_threshold; | |||
| size_t _buffer_enqueue_threshold; | |||
| DEFINE_PAD_MINUS_SIZE(2, DEFAULT_CACHE_LINE_SIZE, 3 * sizeof(size_t)); | |||
| // SATB is only active during marking. Enqueuing is not done when inactive. | |||
There was a problem hiding this comment.
I would prefer to avoid the double-negation in the second sentence.
There was a problem hiding this comment.
Sure. Changing it to
// SATB is only active during marking. Enqueuing is only done when active.
| @@ -138,9 +150,11 @@ class SATBMarkQueueSet: public PtrQueueSet { | |||
| // buffer; the leading entries may be excluded due to filtering. | |||
| bool apply_closure_to_completed_buffer(SATBBufferClosure* cl); | |||
|
|
|||
| // When active, add obj to queue by calling enqueue_known_active. | |||
| void enqueue(SATBMarkQueue& queue, oop obj) { | |||
| if (queue.is_active()) enqueue_known_active(queue, obj); | |||
There was a problem hiding this comment.
Maybe something for a different CR and probably pre-existing: enqueue() in some contexts is already called with queue.is_active() == true. I.e. this check is superfluous sometimes, particularly when called for object arrays.
There was a problem hiding this comment.
I was planning to look at that later. I filed a reminder: https://bugs.openjdk.java.net/browse/JDK-8258607
|
@kimbarrett This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
|
/integrate |
openjdk
bot
added
integrated
|
@kimbarrett Pushed as commit 00d80fd. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
Please review this change to the PtrQueue hierarchy, moving the support for
"active" queues and qset to SATBMarkQueue[Set], which is the only user of
this feature. Other classes derived from PtrQueue[Set] currently work
around or ignore this feature. This change removes it from consideration
entirely for those other classes.
Testing:
mach5 tier1
local (linux-x64) hotspot:tier1 with -XX:+UseShenandoahGC
In the process of doing this refactoring I noticed that some of the
vmStructs support around PtrQueue[Set] didn't get moved from G1 to shared,
and the G1 parts are incomplete. Filed JDK-8258581 to address that.
Progress
Issue
Reviewers
Download
$ git fetch https://git.openjdk.java.net/jdk pull/1820/head:pull/1820$ git checkout pull/1820