Extract crac functionality into OS-agnostic files #84

wkia · 2023-06-16T10:20:35Z

CRaC-related functionality is moved to crac*.hpp/cpp files, and now it is in crac class instead of os.

Progress

Change must not contain extraneous whitespace

Reviewers

Radim Vansa (@rvansa - Committer) ⚠️ Review applies to 780d889b
Anton Kozlov (@AntonKozlov - Reviewer) ⚠️ Review applies to e3c180f7

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/crac.git pull/84/head:pull/84
$ git checkout pull/84

Update a local copy of the PR:
$ git checkout pull/84
$ git pull https://git.openjdk.org/crac.git pull/84/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 84

View PR using the GUI difftool:
$ git pr show -t 84

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/crac/pull/84.diff

Webrev

Link to Webrev Comment

bridgekeeper · 2023-06-16T10:22:20Z

👋 Welcome back rmarchenko! A progress list of the required criteria for merging this PR into crac will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

wkia · 2023-06-16T10:23:06Z

@AntonKozlov @rvansa
Hi,
As we discussed before, I extracted crac* files. Hopefully it will help with futher java version upgrade and win/mac build.

openjdk · 2023-06-16T10:26:11Z

@wkia This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

Extract crac functionality into OS-agnostic files

Reviewed-by: rvansa, akozlov

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been no new commits pushed to the crac branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@rvansa, @AntonKozlov) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

mlbridge · 2023-06-16T10:31:10Z

Webrevs

rvansa

LGTM, I've checked the stuff moved from os_linux.cpp and it matches.

src/hotspot/share/runtime/arguments.cpp

AntonKozlov

I would propose "Extract crac functionality into OS-agnostic files" as the title and eventual commit message, or something like that.

LGTM in the rest! Thank you.

rvansa · 2023-06-19T14:11:37Z

@wkia Integrating #53 regrettably brought a conflict, shouldn't be difficult to resolve, though. Thanks!

openjdk · 2023-06-20T09:24:55Z

@wkia Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

rvansa

LGTM, thanks!

wkia · 2023-06-27T16:32:41Z

@jankratochvil Hi, could you review this regarding your recent changes, please?

commit 5d2fe34 ../../src/hotspot/share/runtime/arguments.cpp: In static member function 'static jint Arguments::finalize_vm_init_args(bool)': ../../src/hotspot/share/runtime/arguments.cpp:3235:28: error: 'crac' has not been declared 3235 | if (CRaCCheckpointTo && !crac::prepare_checkpoint()) { | ^~~~ ../../src/hotspot/share/prims/jvm.cpp: In function '_jobjectArray* JVM_Checkpoint(JNIEnv*, jarray, jobjectArray, jboolean, jlong)': ../../src/hotspot/share/prims/jvm.cpp:3853:16: error: 'crac' has not been declared 3853 | Handle ret = crac::checkpoint(fd_arr, obj_arr, dry_run, jcmd_stream, CHECK_NULL); | ^~~~ ../../src/hotspot/share/services/management.cpp: In function 'jlong get_long_attribute(jmmLongAttribute)': ../../src/hotspot/share/services/management.cpp:957:12: error: 'crac' has not been declared 957 | return crac::restore_start_time(); | ^~~~ ../../src/hotspot/share/services/management.cpp:961:21: error: 'crac' has not been declared 961 | jlong ticks = crac::uptime_since_restore(); | ^~~~ Remove a no longer used declaration of os::Linux::checkpoint_restore().

jankratochvil · 2023-06-28T03:19:06Z

@jankratochvil Hi, could you review this regarding your recent changes, please?

wkia#1

Fix compilation errors:

openjdk · 2023-06-28T08:27:16Z

⚠️ @wkia This pull request contains merges that bring in commits not present in the target repository. Since this is not a "merge style" pull request, these changes will be squashed when this pull request in integrated. If this is your intention, then please ignore this message. If you want to preserve the commit structure, you must change the title of this pull request to Merge <project>:<branch> where <project> is the name of another project in the OpenJDK organization (for example Merge jdk:master).

wkia · 2023-07-03T15:29:26Z

/integrate

openjdk · 2023-07-03T15:30:18Z

@wkia
Your change (at version e733914) is now ready to be sponsored by a Committer.

AntonKozlov · 2023-07-03T17:23:43Z

Thank you! LGTM

/sponsor

openjdk · 2023-07-03T17:26:22Z

Going to push as commit f0cdb1f.
Since your change was applied there has been 1 commit pushed to the crac branch:

c48ccf5: PID adjustment on checkpoint

Your commit was automatically rebased without conflicts.

openjdk · 2023-07-03T17:26:34Z

@AntonKozlov @wkia Pushed as commit f0cdb1f.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Refactoring - extracted crac* files

d262cce

Removing trailing spaces

7b169d9

openjdk bot added ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jun 16, 2023

rvansa approved these changes Jun 16, 2023

View reviewed changes

AntonKozlov reviewed Jun 16, 2023

View reviewed changes

src/hotspot/share/runtime/arguments.cpp Outdated Show resolved Hide resolved

Roman Marchenko added 2 commits June 16, 2023 13:12

Getting rid of crac::Linux

97faf00

Merge branch 'crac' into crac-extract

e3c180f

AntonKozlov approved these changes Jun 19, 2023

View reviewed changes

wkia changed the title ~~Refactoring - extracted crac files~~ Extract crac functionality into OS-agnostic files Jun 19, 2023

AntonKozlov approved these changes Jun 19, 2023

View reviewed changes

Merge branch 'crac' into crac-extract

780d889

wkia force-pushed the crac-extract branch from 40db7d6 to 780d889 Compare June 20, 2023 09:23

wkia requested a review from rvansa June 21, 2023 08:01

rvansa approved these changes Jun 21, 2023

View reviewed changes

Merge branch 'crac' into crac-extract

5d2fe34

Merge pull request #1 from jankratochvil/crac-extract-jan

c49a3a8

Fix compilation errors:

wkia and others added 2 commits June 29, 2023 15:41

Merge branch 'openjdk:crac' into crac-extract

4135078

Merge branch 'master' into crac-extract

e733914

openjdk bot added the sponsor Pull request is ready to be sponsored label Jul 3, 2023

openjdk bot added the integrated Pull request has been integrated label Jul 3, 2023

openjdk bot closed this Jul 3, 2023

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review sponsor Pull request is ready to be sponsored labels Jul 3, 2023

wkia deleted the crac-extract branch July 4, 2023 07:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract crac functionality into OS-agnostic files #84

Extract crac functionality into OS-agnostic files #84

wkia commented Jun 16, 2023 •

edited by openjdk bot

Loading

bridgekeeper bot commented Jun 16, 2023

wkia commented Jun 16, 2023 •

edited

Loading

openjdk bot commented Jun 16, 2023 •

edited

Loading

mlbridge bot commented Jun 16, 2023 •

edited

Loading

rvansa left a comment

AntonKozlov left a comment

rvansa commented Jun 19, 2023

openjdk bot commented Jun 20, 2023

rvansa left a comment

wkia commented Jun 27, 2023

jankratochvil commented Jun 28, 2023

openjdk bot commented Jun 28, 2023

wkia commented Jul 3, 2023

openjdk bot commented Jul 3, 2023

AntonKozlov commented Jul 3, 2023

openjdk bot commented Jul 3, 2023

openjdk bot commented Jul 3, 2023

Extract crac functionality into OS-agnostic files #84

Extract crac functionality into OS-agnostic files #84

Conversation

wkia commented Jun 16, 2023 • edited by openjdk bot Loading

Progress

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Jun 16, 2023

wkia commented Jun 16, 2023 • edited Loading

openjdk bot commented Jun 16, 2023 • edited Loading

mlbridge bot commented Jun 16, 2023 • edited Loading

Webrevs

rvansa left a comment

Choose a reason for hiding this comment

AntonKozlov left a comment

Choose a reason for hiding this comment

rvansa commented Jun 19, 2023

openjdk bot commented Jun 20, 2023

rvansa left a comment

Choose a reason for hiding this comment

wkia commented Jun 27, 2023

jankratochvil commented Jun 28, 2023

openjdk bot commented Jun 28, 2023

wkia commented Jul 3, 2023

openjdk bot commented Jul 3, 2023

AntonKozlov commented Jul 3, 2023

openjdk bot commented Jul 3, 2023

openjdk bot commented Jul 3, 2023

wkia commented Jun 16, 2023 •

edited by openjdk bot

Loading

wkia commented Jun 16, 2023 •

edited

Loading

openjdk bot commented Jun 16, 2023 •

edited

Loading

mlbridge bot commented Jun 16, 2023 •

edited

Loading