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

perf(vm): Fix VM performance regression on CI loadtest #2782

Merged
merged 9 commits into from
Sep 4, 2024

Conversation

slowli
Copy link
Contributor

@slowli slowli commented Sep 2, 2024

What ❔

Fixes VM performance regression on CI loadtest introduced in #2760.

Why ❔

Changes in the VM interface made the VM eagerly clone compressed bytecodes if compression hasn't failed. Compressed bytecodes aren't used during sandboxed VM execution in the API server (the sandbox only checks that compression is successful).For new VMs, bytecodes can be borrowed from the VM state, which is what this PR does using Cow.

Checklist

  • PR title corresponds to the body of PR (we generate changelog entries from PRs).
  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • Code has been formatted via zk fmt and zk lint.

@slowli slowli force-pushed the aov-pla-1024-simplify-vminterface-fix-perf branch 3 times, most recently from 50ff4cd to a8838eb Compare September 3, 2024 14:55
@slowli slowli force-pushed the aov-pla-1024-simplify-vminterface-fix-perf branch from a8838eb to b42ca4a Compare September 4, 2024 06:49
@slowli slowli force-pushed the aov-pla-1024-simplify-vminterface-fix-perf branch from e6beefe to 7ce9332 Compare September 4, 2024 13:47
Copy link
Contributor

github-actions bot commented Sep 4, 2024

No performance difference detected (anymore)

@slowli slowli marked this pull request as ready for review September 4, 2024 14:23
@slowli slowli requested a review from a team as a code owner September 4, 2024 14:23
@slowli
Copy link
Contributor Author

slowli commented Sep 4, 2024

I haven't managed to find any reason for performance regression in recent PRs, and after reverting them, it looks like the load test sometimes fails even when these changes are fully reverted. Thus, I've cheated decided to change load test params instead, so that its load is lesser and more even.

Not sure whether it's just wishful thinking, but the changes in the load test config seem to make the load test more stable (decreasing the number of accounts and concurrent API requests; increasing the number of in-flight txs and randomizing polling interval between polling tx status).

@Deniallugo Deniallugo added this pull request to the merge queue Sep 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 4, 2024
@slowli slowli added this pull request to the merge queue Sep 4, 2024
Merged via the queue into main with commit bc0d7d5 Sep 4, 2024
55 of 56 checks passed
@slowli slowli deleted the aov-pla-1024-simplify-vminterface-fix-perf branch September 4, 2024 16:17
github-merge-queue bot pushed a commit that referenced this pull request Sep 5, 2024
🤖 I have created a release *beep* *boop*
---


##
[24.24.0](core-v24.23.0...core-v24.24.0)
(2024-09-05)


### Features

* conditional cbt l1 updates
([#2748](#2748))
([6d18061](6d18061))
* **eth-watch:** do not query events from earliest block
([#2810](#2810))
([1da3f7e](1da3f7e))
* **genesis:** Validate genesis config against L1
([#2786](#2786))
([b2dd9a5](b2dd9a5))
* Integrate tracers and implement circuits tracer in vm2
([#2653](#2653))
([87b02e3](87b02e3))
* Move prover data to
/home/popzxc/workspace/current/zksync-era/prover/data
([#2778](#2778))
([62e4d46](62e4d46))
* Remove prover db from house keeper
([#2795](#2795))
([85b7346](85b7346))
* **vm-runner:** Implement batch data prefetching
([#2724](#2724))
([d01840d](d01840d))
* **vm:** Extract batch executor to separate crate
([#2702](#2702))
([b82dfa4](b82dfa4))
* **vm:** Simplify VM interface
([#2760](#2760))
([c3bde47](c3bde47))
* **zk_toolbox:** add multi-chain CI integration test
([#2594](#2594))
([05c940e](05c940e))


### Bug Fixes

* **config:** Do not panic for observability config
([#2639](#2639))
([1e768d4](1e768d4))
* **core:** Batched event processing support for Reth
([#2623](#2623))
([958dfdc](958dfdc))
* return correct witness inputs
([#2770](#2770))
([2516e2e](2516e2e))
* **tee-prover:** increase retries to reduce spurious alerts
([#2776](#2776))
([4fdc806](4fdc806))
* **tee-prover:** mitigate panic on redeployments
([#2764](#2764))
([178b386](178b386))
* **tee:** lowercase enum TEE types
([#2798](#2798))
([0f2f9bd](0f2f9bd))
* **vm-runner:** Fix statement timeouts in VM playground
([#2772](#2772))
([d3cd553](d3cd553))


### Performance Improvements

* **vm:** Fix VM performance regression on CI loadtest
([#2782](#2782))
([bc0d7d5](bc0d7d5))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: zksync-era-bot <zksync-era-bot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants