Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

refactor(pass-style): faster passStyleOf #2716

Merged
merged 2 commits into from
Feb 13, 2025
Merged

Conversation

erights
Copy link
Contributor

@erights erights commented Feb 11, 2025

Closes: #XXXX
Refs: Agoric/agoric-sdk#10975 Agoric/agoric-sdk#10982

Description

Using the benchmark tool from #10975 by @muhammadahmadasifbhatti as modified by #10982 to produce flamegraphs (interactive in vscode) I iterated on that benchmark test which already had very specific simple examples exercising passStyleOf. Together with the snippet at https://github.com/Agoric/agoric-sdk/blob/acbb5da3c5a52bab8db319fd696aed70146f9a89/.github/actions/restore-node/action.yml#L156 (which @gibson042 brought to my attention)

$ scripts/get-packed-versions.sh ~/endo | scripts/resolve-versions.sh
$ yarn install && yarn build
$ cd packages/benchmark-test
$ yarn bench

and then clicking on the latest *.cpuprofile file, assuming you've already installed the https://marketplace.visualstudio.com/items?itemName=ms-vscode.vscode-js-profile-flame vscode extension.

I was able to iterate and tinker on possible improvements, to see what made how much of a difference. This PR has the improvements to passStyleOf that I and my reviewers have came up with so far.

@gibson042 suggested the main technique used: to avoid redundant checking by breaking up assertValid so we could still running the checks of canBeValid twice.

Security Considerations

I claim that this PR is a pure refactoring. If true, none. Reviewers, please review with a skeptical eye. passStyleOf is security critical, so any observable difference might lead to an opportunity for an adversary.

Scaling Considerations

Even though I iterated on a specialized benchmark exercising passStyleOf, I believe I only made changes that would also be an improvement for the typical cases.

Documentation Considerations

If this is a pure refactor, none.

Testing Considerations

If this is a pure refactor, none. In fact, this PR did not need to change any tests, providing some weak evidence that it is indeed a pure refactor.

Compatibility Considerations

If this is a pure refactor, none.

Upgrade Considerations

If this is a pure refactor, none.

@erights erights self-assigned this Feb 11, 2025
@erights erights changed the title refactor(pass-style,ses): slightly faster harden & passStyleOf refactor(pass-style): faster passStyleOf Feb 12, 2025
@erights erights marked this pull request as ready for review February 12, 2025 02:28
@erights
Copy link
Contributor Author

erights commented Feb 12, 2025

Ready for review!

@erights erights requested a review from gibson042 February 12, 2025 02:29
@erights erights changed the title refactor(pass-style): faster passStyleOf refactor(pass-style): faster passStyleOf Feb 12, 2025
@erights erights force-pushed the markm-faster-passStyleOf branch from 83a04ac to f5d7897 Compare February 13, 2025 22:02
@erights erights enabled auto-merge (squash) February 13, 2025 22:03
@erights erights merged commit bb2b899 into master Feb 13, 2025
15 checks passed
@erights erights deleted the markm-faster-passStyleOf branch February 13, 2025 22:13
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants