feat(swingset-runner): Add cpu/heap profiling support #11400
Conversation
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
Deploying agoric-sdk with
|
| Latest commit: |
a9a16ed
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://317675e4.agoric-sdk.pages.dev |
| Branch Preview URL: | https://usman-swingset-runner-profil.agoric-sdk.pages.dev |
gibson042
reviewed
May 22, 2025
packages/swingset-runner/src/main.js
Outdated
| @@ -368,6 +377,56 @@ export async function main() { | |||
| case '--sbench': | |||
| swingsetBenchmarkDriverPath = argv.shift(); | |||
| break; | |||
| case '--cpu-profile-output': | |||
| cpuProfileFilePath = path.resolve(String(argv.shift())); | |||
There was a problem hiding this comment.
Suggested change
| cpuProfileFilePath = path.resolve(String(argv.shift())); | |
| cpuProfileFilePath = path.resolve(argv.shift()); |
packages/swingset-runner/src/main.js
Outdated
| case '--cpu-profile-output': | ||
| cpuProfileFilePath = path.resolve(String(argv.shift())); | ||
| await new Promise(resolve => { | ||
| const parentDirectory = path.dirname(String(cpuProfileFilePath)); |
There was a problem hiding this comment.
Suggested change
| const parentDirectory = path.dirname(String(cpuProfileFilePath)); | |
| const parentDirectory = path.dirname(cpuProfileFilePath); |
packages/swingset-runner/src/main.js
Outdated
| const parentDirectory = path.dirname(String(cpuProfileFilePath)); | ||
| fs.access(parentDirectory, fs.constants.F_OK, err => { | ||
| if (err) { | ||
| console.warn( |
There was a problem hiding this comment.
This should throw rather than warn. Also, wouldn't the whole thing be easier with fs/promises?
try {
const parentDirectory = path.dirname(cpuProfileFilePath);
const dir = await fsp.opendir(parentDirectory);
dir.closeSync();
if (!inspectorSession) {
await initializeInspectorSession();
}
} catch (cause) {
throw Error(`Can't record CPU profile at ${cpuProfileFilePath}`, { cause });
}
packages/swingset-runner/src/main.js
Outdated
| case '--heap-sampling-interval': | ||
| samplingInterval = Number(argv.shift()); | ||
| if (Number.isNaN(samplingInterval) || samplingInterval < 1) | ||
| console.warn( |
| config.devices.bridge = { | ||
| sourceSpec: bridge.srcPath, | ||
| }; | ||
| config.devices.bridge = { sourceSpec: bridge.srcPath }; |
There was a problem hiding this comment.
Man, I'd love to see this reimplemented on the cosmic-swingset test kit too!
I have added the |
gibson042
requested changes
May 24, 2025
There was a problem hiding this comment.
This does not render well; the line breaks are all collapsed.
| @@ -303,7 +303,8 @@ export async function main() { | |||
| logStats = true; | |||
| break; | |||
| case '--logtag': | |||
| logTag = argv.shift(); | |||
| nextArg = argv.shift(); | |||
| nextArg && (logTag = nextArg); | |||
There was a problem hiding this comment.
If --logtag '', is invalid, it should be rejected with an error rather than silently ignored.
| @@ -333,11 +334,13 @@ export async function main() { | |||
| doDumps = true; | |||
| break; | |||
| case '--dumpdir': | |||
| dumpDir = argv.shift(); | |||
| nextArg = argv.shift(); | |||
| nextArg && (dumpDir = nextArg); | |||
| doDumps = true; | ||
| break; | ||
| case '--dumptag': | ||
| dumpTag = argv.shift(); | ||
| nextArg = argv.shift(); | ||
| nextArg && (dumpTag = nextArg); |
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.
Description
This PR adds support for generating cpu and/or heap profiles in the swingset runner using the engine's native inspector
Also fixed some typing errors (not all of them)
Security Considerations
None
Scaling Considerations
None
Documentation Considerations
Added help messages for the added flags
Testing Considerations
Tested by generating profiles
Upgrade Considerations
None