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.

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

Swingset startup & benchmark plumbing for node/v8 per-vat profiling & debugging #8491

Merged
merged 1 commit into from
Dec 8, 2023

Conversation

FUDCo
Copy link
Contributor

@FUDCo FUDCo commented Nov 1, 2023

Closes #8490

@FUDCo FUDCo added enhancement New feature or request SwingSet package: SwingSet performance Performance related issues labels Nov 1, 2023
@FUDCo FUDCo requested a review from warner November 1, 2023 19:43
@FUDCo FUDCo self-assigned this Nov 1, 2023
@FUDCo FUDCo force-pushed the refactor-bootstrap-test-support branch from 1995bde to 3f9f8ba Compare November 18, 2023 01:02
Base automatically changed from refactor-bootstrap-test-support to master November 18, 2023 01:59
@FUDCo FUDCo force-pushed the control-profiling branch 2 times, most recently from 0374b46 to 4cc5873 Compare November 18, 2023 02:43
Copy link
Member

@warner warner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great

packages/benchmark/doc/benchmarkerator.md Show resolved Hide resolved
packages/benchmark/doc/benchmarkerator.md Show resolved Hide resolved
packages/benchmark/doc/benchmarkerator.md Show resolved Hide resolved
packages/benchmark/doc/benchmarkerator.md Show resolved Hide resolved
packages/benchmark/doc/benchmarkerator.md Show resolved Hide resolved
packages/benchmark/src/benchmarkerator.js Show resolved Hide resolved
packages/benchmark/src/benchmarkerator.js Show resolved Hide resolved
packages/benchmark/doc/benchmarkerator.md Show resolved Hide resolved
packages/benchmark/src/benchmarkerator.js Show resolved Hide resolved
Comment on lines +15 to +21
if (profileVats.includes(vatID)) {
args.push('--cpu-prof');
args.push('--cpu-prof-interval');
args.push('100');
args.push('--cpu-prof-name');
args.push(`CPU.${nameDisplayArg}.cpuprofile`);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (profileVats.includes(vatID)) {
args.push('--cpu-prof');
args.push('--cpu-prof-interval');
args.push('100');
args.push('--cpu-prof-name');
args.push(`CPU.${nameDisplayArg}.cpuprofile`);
}
if (profileVats.includes(vatID)) {
// cf. https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_282
const fsNameDisplay = nameDisplayArg.replaceAll(':', '-').replaceAll(/[^a-z0-9._-]/gi, '_');
args.push('--cpu-prof');
args.push('--cpu-prof-interval');
args.push('100');
args.push('--cpu-prof-name');
args.push(`vat-${fsNameDisplay}.cpuprofile`);
}

(or similar)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all of your doc suggestions are excellent, but I was literally seconds from tagging the PR for merge when your comments came and I'm reluctant to hold up the train at this stage (since there are a couple of other things waiting for it). So my plan is to land this now as is and incorporate your proposed doc improvements as a follow-on PR.

@FUDCo FUDCo force-pushed the control-profiling branch from 4cc5873 to b680821 Compare December 8, 2023 01:43
@FUDCo FUDCo added the automerge:rebase Automatically rebase updates, then merge label Dec 8, 2023
@mergify mergify bot merged commit 354c092 into master Dec 8, 2023
76 checks passed
@mergify mergify bot deleted the control-profiling branch December 8, 2023 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge enhancement New feature or request performance Performance related issues SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SwingSet startup plumbing and benchmark framework support for v8 profiling of vats
3 participants