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: use vec_map paging mmu for executor memory #1461

Merged
merged 30 commits into from
Sep 4, 2024
Merged

Conversation

tqn
Copy link
Contributor

@tqn tqn commented Aug 31, 2024

Some areas to potentially improve:

  • memory_checkpoint always allocates as much space (for page pointers) as the largest key (page address). This is not necessary. Perhaps it should be a VecDeque of Pages along with its least address, or perhaps a HashMap or BTreeMap would do better. This choice probably depends on the shard size and memory access patterns.
  • memory_checkpoint is PagedMemory<Option<MemoryRecord>> which is internally stored as Vec<Option<Option<MemoryRecord>>>. This is not the best for space.
  • Loads of constant arithmetic at the top of memory.rs. Could be made into const functions taking (the size of) the generic value V.

Copy link

github-actions bot commented Sep 3, 2024

SP1 Performance Test Results

Branch: tqn/perf-executor-slowdown
Commit: b2aaa89b
Author: tqn

program cycles execute (mHz) core (kHZ) compress (KHz) time success
fibonacci 11291 0.02 5.79 0.15 1m17s
ssz-withdrawals 2757356 3.42 59.12 21.99 2m53s
tendermint 12593597 4.69 151.53 93.81 3m40s

@tqn tqn marked this pull request as ready for review September 4, 2024 04:09
crates/core/executor/src/memory.rs Outdated Show resolved Hide resolved
crates/core/executor/src/state.rs Show resolved Hide resolved
crates/core/executor/src/memory.rs Outdated Show resolved Hide resolved
@tqn tqn merged commit 98d100c into dev Sep 4, 2024
10 checks passed
@tqn tqn deleted the tqn/perf-executor-slowdown branch September 4, 2024 22:30
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.

2 participants