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

Refactor GC CRIU Routines #17145

Merged
merged 1 commit into from
Apr 17, 2023
Merged

Conversation

RSalman
Copy link
Contributor

@RSalman RSalman commented Apr 11, 2023

Update GC CRIU checkpoint and restore routines based on refactoring done in upstream OMR: eclipse/omr#6947.

  • Replace configuration reinitializeGCThreadCountForRestore and adjustGCThreadCountForCheckpoint calls with single reinitializeForRestore call.
  • Invoke the dispatcher directly to expand/contract thread pool
  • Update the GC thread count in j9gc_reinitializeDefaults

Depends on eclipse/omr#6947

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

@RSalman
Copy link
Contributor Author

RSalman commented Apr 11, 2023

@amicic could you please have a look

@RSalman RSalman changed the title Refactor GC CRIU checkpoint and restore routines Refactor GC CRIU routines Apr 11, 2023
@RSalman RSalman changed the title Refactor GC CRIU routines Refactor GC CRIU Routines Apr 11, 2023
@amicic
Copy link
Contributor

amicic commented Apr 11, 2023

should we expand the logic on dispatcher too and rename expandThreadPool to reinitializeForRestore (and something similar for contract/checkpoint)?

@RSalman
Copy link
Contributor Author

RSalman commented Apr 11, 2023

Yup, it would be nice to align dispatcher functions as well

@RSalman RSalman force-pushed the criu-modron-refac branch 2 times, most recently from 1888ea3 to 96a33d9 Compare April 11, 2023 21:02
@RSalman
Copy link
Contributor Author

RSalman commented Apr 11, 2023

I've made the dispatcher changes, OMR needs to be updated as well

@@ -1131,6 +1131,9 @@ j9gc_reinitialize_for_restore(J9VMThread *vmThread, const char **nlsMsgFormat)
J9JavaVM *vm = vmThread->javaVM;
J9MemoryManagerVerboseInterface *mmFuncTable = (J9MemoryManagerVerboseInterface *)vm->memoryManagerFunctions->getVerboseGCFunctionTable(vm);

Assert_MM_true(NULL != extensions->getGlobalCollector());
Assert_MM_true(NULL != extensions->configuration);

PORT_ACCESS_FROM_JAVAVM(vm);

if (!j9gc_reinitializeDefaults(vmThread)) {
Copy link
Contributor

@amicic amicic Apr 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

noticed this is an inter module method, but it does not seem like it has to be
also should it have 'for restore' in the name ?
btw original method is gcInitializeDefaults (no j9 in the name)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

noticed this is an inter module method, but it does not seem like it has to be

Removed from J9MemoryManagerFunctions

also should it have 'for restore' in the name ?
btw original method is gcInitializeDefaults (no j9 in the name)

Changed j9gc_reinitializeDefaults -> gcReinitializeDefaultsForRestore

Update GC CRIU checkpoint and restore routines based on refactoring done
in upstream OMR: eclipse/omr#6947.

- Replace configuration reinitializeGCThreadCountForRestore and
adjustGCThreadCountForCheckpoint calls with single
reinitializeForRestore call.
- Invoke the dispatcher directly to expand/contract thread pool
- Update the GC thread count in j9gc_reinitializeDefaults

Depends on eclipse/omr#6947

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

amicic commented Apr 17, 2023

Jenkins test sanity xlinuxcriu jdk17

@amicic
Copy link
Contributor

amicic commented Apr 17, 2023

Jenkins compile win jdk8

@amicic amicic added comp:gc criu Used to track CRIU snapshot related work labels Apr 17, 2023
@amicic amicic merged commit 7bff45e into eclipse-openj9:master Apr 17, 2023
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

2 participants