- 
                Notifications
    You must be signed in to change notification settings 
- Fork 6.1k
8370203 - Add jcmd AOT.end_recording diagnostic command #27965
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
base: master
Are you sure you want to change the base?
Conversation
| 👋 Welcome back macarte! A progress list of the required criteria for merging this PR into  | 
| ❗ This change is not yet ready to be integrated. | 
| @macarte The following labels will be automatically applied to this pull request: 
 When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command. | 
|  | ||
| bool AOTMetaspace::is_recording_preimage_static_archive() { | ||
| if (CDSConfig::is_dumping_preimage_static_archive()) { | ||
| return _preimage_static_archive_dumped == 0; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to use an acquiring load here? i.e. can the read here and the cmpxchg below happen in different threads?
n.b. I'm not just thinking about the behaviour when this patch makes the DCmd available but also what happens when we supplement it with the MXBean interface to end recordings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's okay, as ending the recording is controlled by the cmpxchg, even if two threads think the recording is still going on, only one call to end the recording will work, and if the threads both check whether the recording has completed they will both see that it has (regardless of which thread 'won')
| return; | ||
| } | ||
|  | ||
| output()->print_cr("Error! Failed to end recording"); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an error recorded or anything further that could be included for the error case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is not, currently, the aotMetaspace method throws an exception if something goes wrong. I added this line in case the flow in aotMetaspace changes. The logic here is that if we asked the recording to stop, but it's still recording then we report to the user that 'we failed to stop the recording'
Add jcmd AOT.end_recording diagnostic command. When this command is issued, a targeted JVM that is currently recording AOT information will stop recording. Existing functionality is preserved: when stopped the JVM will create the required artifacts based on the execution mode. Conveniently as the application running on the JVM has not stopped (as was previously the only way to stop recording), the application will resume execution after the artifacts have been generated.
The command will report back to the user one of the following messages depending on the state of the JVM:
It follows that issues the command to a JVM that is recording, twice in succession, should (baring internal errors) would produce the following two responses:
Passes tier1 on linux (x64) and windows (x64)
Progress
Issues
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27965/head:pull/27965$ git checkout pull/27965Update a local copy of the PR:
$ git checkout pull/27965$ git pull https://git.openjdk.org/jdk.git pull/27965/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 27965View PR using the GUI difftool:
$ git pr show -t 27965Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27965.diff
Using Webrev
Link to Webrev Comment