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.

Sign up for GitHub

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

Jump to bottom

CRIU resets j.l.VirtualThread.ForkJoinPool.parallelism after restore #17618

Merged
merged 1 commit into from Sep 12, 2023
Merged

CRIU resets j.l.VirtualThread.ForkJoinPool.parallelism after restore #17618

merged 1 commit into from Sep 12, 2023

Conversation

JasonFengJ9
Copy link
Member

@JasonFengJ9 JasonFengJ9 commented Jun 19, 2023 •
edited

CRIU resets j.l.VirtualThread.ForkJoinPool.parallelism after restore

If there is no system property jdk.virtualThreadScheduler.parallelism set at VM startup, j.l.VirtualThread.ForkJoinPool.parallelism value is reset to match Runtime.getRuntime().availableProcessors() after CRIU
restore;
Added VM_VMHelpers methods getStaticFieldObject/findinstanceFieldOffset/setObjectFieldI32;
Changed hashClassTableAt to peekClassHashTable;
Minor refactoring;
Added tests.

Signed-off-by: Jason Feng fengj@ca.ibm.com

@JasonFengJ9 JasonFengJ9 added comp:vm criu Used to track CRIU snapshot related work jdk20 labels Jun 19, 2023
@JasonFengJ9 JasonFengJ9 requested a review from tajila June 19, 2023 12:21
tajila
tajila reviewed Jun 19, 2023
View reviewed changes
runtime/vm/CRIUHelpers.cpp Show resolved Hide resolved
runtime/vm/CRIUHelpers.cpp Show resolved Hide resolved
runtime/vm/CRIUHelpers.cpp Outdated Show resolved Hide resolved
@JasonFengJ9 JasonFengJ9 mentioned this pull request Jun 20, 2023
Closed
@JasonFengJ9 JasonFengJ9 force-pushed the updatefjpparallelism branch 3 times, most recently from 4085b76 to bd167d1 Compare June 22, 2023 01:39
@JasonFengJ9 JasonFengJ9 force-pushed the updatefjpparallelism branch 2 times, most recently from a29effa to a76f932 Compare July 28, 2023 14:37
tajila
tajila reviewed Aug 29, 2023
View reviewed changes
runtime/nls/j9vm/j9vm.nls Outdated Show resolved Hide resolved
@JasonFengJ9
CRIU resets j.l.VirtualThread.ForkJoinPool.parallelism after restore
ac50591 
If there is no system property jdk.virtualThreadScheduler.parallelism
set at VM startup, j.l.VirtualThread.ForkJoinPool.parallelism value is
reset to match Runtime.getRuntime().availableProcessors() after CRIU
restore;
Added VM_VMHelpers methods
getStaticFieldObject/findinstanceFieldOffset/setObjectFieldI32;
Changed hashClassTableAt to peekClassHashTable;
Minor refactoring;
Added tests.

Signed-off-by: Jason Feng <fengj@ca.ibm.com>
@tajila
Copy link
Contributor

tajila commented Sep 11, 2023

Im wondering if we should just use vmconstantpool for this. Im trying to think of a downside, but I think the cost to startup is negligeable

@JasonFengJ9
Copy link
Member Author

Im wondering if we should just use vmconstantpool for this. Im trying to think of a downside, but I think the cost to startup is negligeable

This mimics the earlier modification of java/util/concurrent/atomic/AtomicLong.value. There were discussions about loading it at startup via vmconstantpool #14365 (comment), it was decided not to do so to avoid unnecessary class instances.
In addition, besides minor startup costs, it also adds a dependency on OpenJDK class initialisation at OpenJ9 bootup stage though most of the time is fine.
vmconstantpool does offer a simpler way for the field modification.

@tajila
Copy link
Contributor

tajila commented Sep 11, 2023

I mention it here, because with VirtualThreads ForkJoinPool will certainly be used. I guess it may not be with pre-JDK 21 builds

@tajila
Copy link
Contributor

tajila commented Sep 11, 2023

jenkins compile win jdk8

@tajila
Copy link
Contributor

tajila commented Sep 11, 2023

jenkins test sanity alinux64 jdk21

@tajila tajila merged commit dce0276 into eclipse-openj9:master Sep 12, 2023
6 checks passed
@JasonFengJ9 JasonFengJ9 deleted the updatefjpparallelism branch September 12, 2023 14:32
@JasonFengJ9 JasonFengJ9 mentioned this pull request Sep 12, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tajila tajila tajila approved these changes

Assignees
No one assigned
Labels
comp:vm criu Used to track CRIU snapshot related work jdk20
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@JasonFengJ9 @tajila