Skip to content

Conversation

@hero78119
Copy link
Collaborator

aggregate lookup expression in keccak precompile instead of pass flatten out_evals of raw lookup value.
This make later gkr-iop flow integration easier when integrate into ceno main proving flow

benchmark

with command on research bench server

JEMALLOC_SYS_WITH_MALLOC_CONF=retain:true,metadata_thp:always,thp:always,dirty_decay_ms:-1,muzzy_decay_ms:-1,abort_conf:true cargo bench -p gkr_iop --features jemalloc --bench lookup_keccakf -- --baseline baseline

There shows prover performance keccak precompile regressed which is expected. Because the cost to aggregate them always there, just we do it within chip or outside chip.

Benchmark Median Time (s) Median Change (%)
keccak_lookup_f_4096 0.98618 +29.015% (Performance has regressed)
keccak_lookup_f_8192 1.9001 +23.847% (Performance has regressed)

@hero78119 hero78119 requested a review from spherel June 30, 2025 06:48
@hero78119 hero78119 force-pushed the feat/lookup_aggregate_in_keccak branch from 1876357 to 6e53d94 Compare June 30, 2025 06:49
let mut evaluations = evaluations; // Change evaluations' lifetime.
evaluations.resize_with(circuit.n_evaluations, || &field_type_default);
challenges.resize_with(circuit.n_challenges, || E::random(&mut rng));
challenges.resize_with(2 + circuit.n_challenges, || E::random(&mut rng));
Copy link
Member

Choose a reason for hiding this comment

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

Why is it +2?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

index 0, 1 is for alpha, beta global challenge. Here circuit.n_challenges only counting local challenges, which is 0.
we can make this converation open without Resolve 👍 , so it can be refer from future task

Copy link
Member

@spherel spherel left a comment

Choose a reason for hiding this comment

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

LGTM!

@hero78119 hero78119 added this pull request to the merge queue Jul 1, 2025
Merged via the queue into scroll-tech:master with commit 4c58c75 Jul 1, 2025
4 checks passed
@hero78119 hero78119 deleted the feat/lookup_aggregate_in_keccak branch July 1, 2025 01:54
github-merge-queue bot pushed a commit that referenced this pull request Jul 1, 2025
to support e2e integration, build on top of #971
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