8381561: G1: Cleanup patch in preparation for eager reclamation of flat arrays#30549
8381561: G1: Cleanup patch in preparation for eager reclamation of flat arrays#30549stefank wants to merge 1 commit intoopenjdk:masterfrom
Conversation
|
👋 Welcome back stefank! A progress list of the required criteria for merging this PR into |
|
@stefank 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 2 new commits pushed to the
Please see this link for an up-to-date comparison between the source branch of this pull request and the ➡️ To integrate this PR with the above commit message to the |
| size_t initial_chunk_size = _partial_array_splitter.start(_task_queue, obj_array, nullptr, array_length); | ||
|
|
||
| // Mark objArray klass metadata | ||
| if (_cm_oop_closure->do_metadata()) { |
There was a problem hiding this comment.
This check has been removed. Marking closures should follow the metadata and there's no need to check if it should.
| } | ||
|
|
||
| size_t G1CMTask::start_partial_array_processing(oop obj) { | ||
| size_t G1CMTask::start_partial_array_processing(objArrayOop obj) { |
There was a problem hiding this comment.
Maybe the arg name can be obj_array to emphasize that it's an objArray.
There was a problem hiding this comment.
Maybe.
I prefer obj since it is a more common name. It seems like only this function and its counter part in the full GC named this obj_array. There's almost 50 occurrences that just call them obj.
I also don't like giving local variables multi-word names unless it really helps with the readability and understanding of the function. I don't think it does in this case.
|
I've run this through tier1-3. |
During the review of openjdk/valhalla#2255 there were some discussion about the code structure that was being proposed. To not make structural changes like this in the valhalla branch, I'm going to propose the changes to openjdk/jdk and then the valhalla changes can be kept to a minimum.
This is what the valhalla changes would look like:
stefank/valhalla@lworld_g1_eager_baseline...stefank:valhalla:lworld_g1_eager
WDYT?
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/30549/head:pull/30549$ git checkout pull/30549Update a local copy of the PR:
$ git checkout pull/30549$ git pull https://git.openjdk.org/jdk.git pull/30549/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 30549View PR using the GUI difftool:
$ git pr show -t 30549Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/30549.diff
Using Webrev
Link to Webrev Comment