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

Get the start time of the criu restore process #18680

Merged
merged 1 commit into from
Jan 25, 2024
Merged

Get the start time of the criu restore process #18680

merged 1 commit into from
Jan 25, 2024

Conversation

singh264
Copy link
Contributor

@singh264 singh264 commented Jan 2, 2024 •
edited
Loading

Get the start time of the criu restore process, which is
the parent process of the restored java process.

Issue: #18598
Depends-on: eclipse-omr/omr#7214
Signed-off-by: Amarpreet Singh [email protected]

@singh264
Copy link
Contributor Author

singh264 commented Jan 2, 2024

@tajila requesting your review.

@singh264 singh264 marked this pull request as ready for review January 2, 2024 17:15
@pshipton
Copy link
Member

pshipton commented Jan 2, 2024

Re "Depends-on: eclipse-omr/omr#7201", 7201 is an issue, not a PR.

@pshipton pshipton added comp:vm comp:test criu Used to track CRIU snapshot related work labels Jan 2, 2024
@singh264
Copy link
Contributor Author

singh264 commented Jan 2, 2024

Re "Depends-on: eclipse-omr/omr#7201", 7201 is an issue, not a PR.

I adjusted Depends-on to refer to a PR.

@pshipton pshipton marked this pull request as draft January 2, 2024 17:44
@pshipton
Copy link
Member

pshipton commented Jan 2, 2024

Set as draft until the OMR change is promoted.

@singh264 singh264 force-pushed the openj9_issues_18598 branch 2 times, most recently from aa0cd8e to bc30c53 Compare January 12, 2024 22:28
@singh264
Copy link
Contributor Author

Set as draft until the OMR change is promoted.

The omr PR was merged.

@singh264
Copy link
Contributor Author

@tajila @babsingh @keithc-ca requesting your review.

@singh264 singh264 marked this pull request as ready for review January 12, 2024 23:59
TobiAjila
TobiAjila reviewed Jan 14, 2024
View reviewed changes
runtime/vm/jvminit.c Outdated Show resolved Hide resolved
@keithc-ca keithc-ca self-requested a review January 16, 2024 15:33
@singh264 singh264 force-pushed the openj9_issues_18598 branch 2 times, most recently from 4d1d800 to 9726368 Compare January 16, 2024 19:59
@singh264
Copy link
Contributor Author

@tajila re-requesting your review.

tajila
tajila reviewed Jan 16, 2024
View reviewed changes
runtime/vm/jvminit.c Outdated Show resolved Hide resolved
@singh264 singh264 force-pushed the openj9_issues_18598 branch 2 times, most recently from 7f49a1c to e8dbbbe Compare January 17, 2024 16:31
@singh264
Copy link
Contributor Author

@tajila re-requesting your review.

@tajila
Copy link
Contributor

tajila commented Jan 17, 2024

What is the point of those two methods anyway? I only see uses in tests.

The purpose of getCRIUProcessRestoreStartTime() is to return the "startup time" of the restored process. The most accurate measureent of this must inculde the time taken by criu restore. Thats why we have criu in the name.

The purpose getLastRestoreTime() is to determine when the JVM process is resumed. Currently, this is only needed for debugging purposes. But, it is conceivable that in the future the time taken by criu may become an important metric to report. Taking the difference between the two would give us this info.

@singh264 singh264 changed the title Set restore time to the criu restore process start time Get the start time of the criu restore process Jan 18, 2024
@singh264
Copy link
Contributor Author

@keithc-ca re-requesting your review.

@singh264 singh264 force-pushed the openj9_issues_18598 branch 2 times, most recently from bcec276 to 89728ce Compare January 19, 2024 18:19
@singh264
Copy link
Contributor Author

@keithc-ca re-requesting your review.

@singh264
Copy link
Contributor Author

@tajila all comments are resolved.

keithc-ca
keithc-ca requested changes Jan 23, 2024
View reviewed changes
runtime/oti/j9nonbuilder.h Outdated Show resolved Hide resolved
test/functional/cmdLineTests/criu/src/org/openj9/criu/TimeChangeTest.java Outdated Show resolved Hide resolved
runtime/vm/j9vm.tdf Outdated Show resolved Hide resolved
test/functional/cmdLineTests/criu/criu_nonPortable.xml Outdated Show resolved Hide resolved
test/functional/cmdLineTests/criu/src/org/openj9/criu/TimeChangeTest.java Outdated Show resolved Hide resolved
runtime/vm/CRIUHelpers.cpp Outdated Show resolved Hide resolved
@singh264 singh264 force-pushed the openj9_issues_18598 branch 2 times, most recently from c49cc07 to 3a22284 Compare January 24, 2024 01:55
@singh264
Copy link
Contributor Author

@keithc-ca re-requesting your review.

@singh264
Copy link
Contributor Author

@keithc-ca re-requesting your review.

@keithc-ca
Copy link
Contributor

Jenkins test sanity xlinux jdk17

@keithc-ca
Copy link
Contributor

The conflict with #18627 will need to be resolved.

@keithc-ca
Copy link
Contributor

For the record, the previous build was https://openj9-jenkins.osuosl.org/job/PullRequest-OpenJ9/4950.

@singh264
Get the start time of the criu restore process
b8c17e8 
Get the start time of the criu restore process, which is
the parent process of the restored java process.

Issue: eclipse-openj9#18598
Depends-on: eclipse-omr/omr#7214
Signed-off-by: Amarpreet Singh <[email protected]>
@singh264
Copy link
Contributor Author

The conflict with #18627 will need to be resolved.

The conflict is resolved.

@keithc-ca
Copy link
Contributor

Jenkins test sanity xlinux jdk17

@keithc-ca keithc-ca merged commit 8c73b1d into eclipse-openj9:master Jan 25, 2024
5 checks passed
@singh264 singh264 mentioned this pull request Jan 29, 2024
Closed
@keithc-ca keithc-ca mentioned this pull request Jan 31, 2024
Merged
This was referenced Feb 6, 2024
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TobiAjila TobiAjila TobiAjila left review comments

@tajila tajila tajila approved these changes

@keithc-ca keithc-ca keithc-ca approved these changes

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

Successfully merging this pull request may close these issues.
5 participants
@singh264 @pshipton @tajila @keithc-ca @TobiAjila