8260309: Shenandoah: Clean up ShenandoahBarrierSet #2202
Conversation
|
👋 Welcome back rkennke! A progress list of the required criteria for merging this PR into |
|
@rkennke 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 21 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
openjdk
bot
added
ready
|
@rkennke To determine the appropriate audience for reviewing this pull request, one or more labels corresponding to different subsystems will normally be applied automatically. However, no automatic labelling rule matches the changes in this pull request. In order to have an "RFR" email sent to the correct mailing list, you will need to add one or more applicable labels manually using the /label pull request command. Applicable Labels
|
openjdk
bot
removed
ready
|
What is up with this PR? There are no recorded changes, and tests are failing ;) |
Weird, I seem to have messed it up. I'll fix it shortly. |
|
@rkennke this pull request can not be integrated into git checkout JDK-8260309
git fetch https://git.openjdk.java.net/jdk master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push |
openjdk
bot
added
merge-conflict
openjdk
bot
removed
the
merge-conflict
It should be good to review, now. |
src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.inline.hpp
Outdated
Show resolved
Hide resolved
|
/integrate |
openjdk
bot
added
integrated
|
@rkennke Since your change was applied there have been 22 commits pushed to the
Your commit was automatically rebased without conflicts. Pushed as commit df33595. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
We collected some cruft in ShenandoahBarrierSet. Time to clean it up.
This fixes/removes a number of includes, fixes some comments and it also removes is_a() and is_aligned() which look like leftovers/requirements from earlier incarnations of the superclass BarrierSet. Using the override keyword would be useful for such situations (btw, are we ok to start using override, nullptr, auto etc in Shenandoah, or do we want to keep it C++ for backporting ease?)
One thing I was not sure about is the ShenandoahHeap* _heap field. Making it const will likely help the compiler avoid repeated access (e.g. in a number of perf-critical paths like the LRB impl). However, maybe we should get rid of the field altogether and make it explicitely using ShenandoahHeap::heap() and avoid repeated access instead of helping the compiler and hoping for the best?
Testing:
Progress
Issue
Reviewers
Download
$ git fetch https://git.openjdk.java.net/jdk pull/2202/head:pull/2202$ git checkout pull/2202