-
Notifications
You must be signed in to change notification settings - Fork 102
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
Keccakf32Memory
: use compute_from
instead of provide_value
#2553
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20
.
Benchmark suite | Current: 4c5e3fd | Previous: bc3bea1 | Ratio |
---|---|---|---|
executor-benchmark/keccak |
24221445626 ns/iter (± 1385068933 ) |
20134690647 ns/iter (± 577397123 ) |
1.20 |
This comment was automatically generated by workflow using github-action-benchmark.
d7fec19
to
1c15647
Compare
Keccakf32Memory
: use compute_from
instead of provide_value
0, | ||
|acc, e| acc ^ e | ||
); | ||
|
||
query |row| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this could be something like
query |row| compute_from_multi(c, row, a, |a_values| {
array::new(array::len(c), |i| {
let x = i / 64;
let z = i % 64;
let limb = z / 32;
let bit_in_limb = z % 32;
// compute c here
})
});
Nice, with |
I realized that most hints are actually not needed; the solver can figure them out. So I removed them. The speed does decrease again though (witgen time for |
This PR changes the usage of
std::prover::provide_value
inKeccakf32Memory
tostd::prover::compute_from
, in the hope of being able to use the JIT for this machine in #2541.Witgen time goes oftest_data/std/keccakf32_memory_test.asm
from 60s to 76s, probably because too many things are being evaluated.This is fixed by 1c15647.