Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unbounded GC Thread Pool Expansion for CRIU Restore #16666

Merged
merged 1 commit into from Feb 10, 2023

Conversation

RSalman
Copy link
Contributor

@RSalman RSalman commented Feb 3, 2023

Initial set of changes to expand the GC thread pool beyond the dispatcher initialization done at VM startup. These changes are required by the consumed OMR GC dispatcher/configuration APIs called for restore.

  • Converted reference obj list iteration count from gcThreadCount to regionExtension->_maxListIndex, this is more correct. This is the only instance in the code where iteration implies the listCount from GC thread count. This inference was not an issue as the lists are inited based on the GC thread count, however now that the thread count may change, it is incorrect to assume the list count from the thread count.
  • Release restore thread's VM access for the duration of reinitializeGCThreadCountForRestore
  • Introduced j9gc_reinitializeDefaults in mminit, this is meant to be the top level init/verify method (e.g parsing/initing GC params based on restore cmd line opts). This is symmetrical restore call to VM startup gcInitializeDefaults call .
  • Introduced setMaxGCThreadCount in Configuration Delegate to simply computation of default gc thread count at restore time

Signed-off-by: Salman Rana salman.rana@ibm.com

@RSalman RSalman force-pushed the criu-v3 branch 2 times, most recently from 2cf4a9e to 717f5af Compare February 3, 2023 16:54
@RSalman
Copy link
Contributor Author

RSalman commented Feb 3, 2023

@amicic @dmitripivkine @tajila please review

@dmitripivkine dmitripivkine added comp:gc criu Used to track CRIU snapshot related work labels Feb 3, 2023
@RSalman RSalman force-pushed the criu-v3 branch 4 times, most recently from 1773c91 to 58f4174 Compare February 6, 2023 18:59
@RSalman RSalman requested a review from amicic February 6, 2023 19:12
Initial set of changes to expand the GC thread pool beyond the
dispatcher initialization done at VM startup. These changes are required
by the consumed OMR GC dispatcher/configuration APIs called for restore.

- Converted reference obj list iteration count from `gcThreadCount` to
`regionExtension->_maxListIndex`, this is more correct. This is the only
instance in the code where iteration implies the listCount from GC
thread count. This inference was not an issue as the lists are inited
based on the GC thread count, however now that the thread count may
change, it is incorrect to assume the list count from the thread count.
- Release restore thread's VM access for the duration of
`reinitializeGCThreadCountForRestore`

Signed-off-by: Salman Rana <salman.rana@ibm.com>
@RSalman
Copy link
Contributor Author

RSalman commented Feb 6, 2023

Latest changes:

  • Introduced j9gc_reinitializeDefaults in mminit, this is meant to be the top level init/verify method (e.g parsing/initing GC params based on restore cmd line opts). This is symmetrical restore call to VM startup gcInitializeDefaults call .
  • Introduced setMaxGCThreadCount in Configuration Delegate to simply computation of default gc thread count at restore time, see related OMR for use.
  • Removed RestoreGCThreads opt from these changes

@amicic
Copy link
Contributor

amicic commented Feb 9, 2023

Jenkins test sanity win jdk11

@amicic
Copy link
Contributor

amicic commented Feb 9, 2023

Jenkins compile aix jdk8

@amicic
Copy link
Contributor

amicic commented Feb 9, 2023

Jenkins test sanity xlinuxcriu jdk17

@amicic
Copy link
Contributor

amicic commented Feb 10, 2023

Jenkins compile aix jdk8

@amicic
Copy link
Contributor

amicic commented Feb 10, 2023

Restarted a failed aix compile, although this seemed like an infra problem. Merging...

@amicic amicic merged commit 282ede7 into eclipse-openj9:master Feb 10, 2023
RSalman added a commit to RSalman/openj9 that referenced this pull request Feb 10, 2023
Move init of `_maximumDefaultNumberOfGCThreads` filed to ctor init list.

Config Delegate's field `_maximumDefaultNumberOfGCThreads` const
decleration was removed in.
eclipse-openj9#16666. This resulted in
DDR compile errors:
```
  [exec] "gc_glue_java/ConfigurationDelegate.hpp", line 67: error: data
member
     [exec]           initializer is not allowed
     [exec]    uintptr_t _maximumDefaultNumberOfGCThreads = 64;
```

Signed-off-by: Salman Rana <salman.rana@ibm.com>
keithc-ca added a commit to keithc-ca/openj9 that referenced this pull request Mar 23, 2023
Required to enable CRIU support since eclipse-openj9#16666.

Signed-off-by: Keith W. Campbell <keithc@ca.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:gc criu Used to track CRIU snapshot related work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants