Skip to content

Cleaning for Concurrent Scavenger initialization - #15610

Merged
amicic merged 1 commit into
eclipse-openj9:masterfrom
dmitripivkine:master
Jul 27, 2022
Merged

Cleaning for Concurrent Scavenger initialization#15610
amicic merged 1 commit into
eclipse-openj9:masterfrom
dmitripivkine:master

Conversation

@dmitripivkine

Copy link
Copy Markdown
Contributor
  • Concurrent Scavenger can be run at any platform where it is enabled,
    there is no extra check required
  • There is no need to enforce usage Software Range Check Barruer on
    platforms where HW is not available
  • add extensions->softwareRangeCheckReadBarrierForced to indicate
    -XXgc:softwareRangeCheckReadBarrier is requested explicitly, stop use
    extensions->softwareRangeCheckReadBarrier for this purpose

Signed-off-by: Dmitri Pivkine Dmitri_Pivkine@ca.ibm.com

@dmitripivkine

Copy link
Copy Markdown
Contributor Author

fixing #15590

@pshipton
pshipton requested a review from amicic July 25, 2022 16:38
@amicic

amicic commented Jul 26, 2022

Copy link
Copy Markdown
Contributor

Introduction of Forced flag and the change of logic seems to be right, but it would be more consistent with the rest of status/control flags related to CS to have the flag at OMR side, since all those flags are already in OMR.

If doing that (touching OMR code), I'd also consider getting rid of one of these 2 flags: softwareRangeCheckReadBarrier or concurrentScavengerHWSupport - they seem to be just a complement of each other. There are also related wrappers to be considered for removal. Not forcing this - think if it makes sense and which one would make more sense to leave and which to remove, if at all.

@dmitripivkine

Copy link
Copy Markdown
Contributor Author

Introduction of Forced flag and the change of logic seems to be right, but it would be more consistent with the rest of status/control flags related to CS to have the flag at OMR side, since all those flags are already in OMR.

I thought about this. I can do it if you insist. The reason I put flag at OpenJ9 side is despite this flag has similar name it relates with Java command line option only. It has nothing to deal with from OMR side.

If doing that (touching OMR code), I'd also consider getting rid of one of these 2 flags: softwareRangeCheckReadBarrier or concurrentScavengerHWSupport - they seem to be just a complement of each other. There are also related wrappers to be considered for removal. Not forcing this - think if it makes sense and which one would make more sense to leave and which to remove, if at all.

I thought about this too. The hidden condition is included to these flags is CS Active (CS Active plus Software Range Check Read Barrier and CS Active plus HW Support) so, we can not say softwareRangeCheckReadBarrier = ! concurrentScavengerHWSupport without check of third condition. There is why I left it as-is. Again, if you insist, I can modify this logic too.
Please let me know what do you think.

@amicic

amicic commented Jul 26, 2022

Copy link
Copy Markdown
Contributor

still think the flag belongs to omr side.
the second part about removing is not that important to change, but you could add a comment for one of the flags it's a complement of the other

- Concurrent Scavenger can be run at any platform where it is enabled,
there is no extra check required
- There is no need to enforce usage Software Range Check Barruer on
platforms where HW is not available
- use extensions->softwareRangeCheckReadBarrierForced to indicate
-XXgc:softwareRangeCheckReadBarrier is requested explicitly, stop use
extensions->softwareRangeCheckReadBarrier for this purpose


Signed-off-by: Dmitri Pivkine <Dmitri_Pivkine@ca.ibm.com>
@dmitripivkine

Copy link
Copy Markdown
Contributor Author

Introduce flag at OMR side: eclipse-omr/omr#6625

@dmitripivkine dmitripivkine added the depends:omr Pull request is dependent on a corresponding change in OMR label Jul 26, 2022
@dmitripivkine

Copy link
Copy Markdown
Contributor Author

Jenkins test sanity xLinux,zLinux jdk11

@amicic
amicic merged commit d4f0a0c into eclipse-openj9:master Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug comp:gc depends:omr Pull request is dependent on a corresponding change in OMR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants