refactor(cli): consolidate JFR capture loop into JfrCaptureSession#170
Merged
Conversation
Four CLI commands (gcwhy, gcscore, gcprofile, zgc) each carried their own ~50-line copy of the same JFR capture lifecycle: tempfile β JFR.start β sleep β JFR.dump β JFR.stop β file-empty check β finally delete tempfile. Subtle drift had crept in: gcprofile passed `duration=` to JFR.start, which races the explicit dump and can yield an empty file (gcwhy already documented this pitfall but the others didn't avoid it). zgc had a unique pre-stop + retry-on-start-fail wrapper. Move the lifecycle behind JfrCaptureSession, returning an AutoCloseable Capture so try-with-resources handles tempfile cleanup. Failures surface as the new checked JfrCaptureFailed; callers translate that into their own exit-code (gcwhy/gcscore/gcprofile keep return-on-fail, zgc keeps exit code 2). Side effects: - gcprofile no longer passes duration= to JFR.start; matches gcwhy/gcscore. - zgc drops its retry-once-on-start-fail loop; the helper does an unconditional pre-stop, which addresses the same leftover-recording case the retry was guarding against. Net change in command files: -269 lines. Signed-off-by: rlaope <piyrw9754@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four CLI commands β
gcwhy,gcscore,gcprofile,zgcβ each carried their own ~50-line copy of the same JFR capture lifecycle: tempfile βJFR.startβ sleep βJFR.dumpβJFR.stopβ file-empty check βfinally delete tempfile. Subtle drift had crept in:gcprofilepassedduration=toJFR.start, which races the explicit dump and can yield an empty file.gcwhyalready documented this pitfall but the others didn't avoid it.zgchad a unique pre-stop + retry-on-start-fail wrapper.This PR moves the lifecycle behind
JfrCaptureSession, returning anAutoCloseableCaptureso try-with-resources handles tempfile cleanup. Failures surface as the new checkedJfrCaptureFailed; callers translate that into their own exit-code convention (gcwhy/gcscore/gcprofile keep return-on-fail, zgc keepsCommandExitException(2)).argus-cli/.../jfr/JfrCaptureSession.java(~111 LOC) andJfrCaptureFailed.java(checked exception)JfrCaptureSessionTestcovers the unreachable-pid failure path (no leftover temp files) and theCapture#closedeletion contractSide effects
gcprofileno longer passesduration=toJFR.start. This fixes a latent race where the JVM auto-stops the recording before the explicit dump.zgcdrops its retry-once-on-start-fail loop. The helper does an unconditional pre-stop of any leftover recording with the same name, which is the case the retry was guarding against.Test plan
:argus-cli:testpasses (incl. newJfrCaptureSessionTest)./gradlew build -x integrationTestpassesargus gcwhy <pid> --duration=10 --format=jsonproduces non-empty JSONargus zgc <pid> --duration=10for a ZGC JVM still produces a verdictargus gcprofile <pid> --duration=10produces an allocation profile