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

Faster benchmarks #386

Closed
wants to merge 1 commit into from
Closed

Conversation

fboemer
Copy link
Contributor

@fboemer fboemer commented Aug 25, 2021

Removes the overhead from due to pausing/resuming in Google benchmarks, which allows removing the iteration count 10. This results in more stable benchmarks and easier control of timings. Note, the EvaluateModSwitch and EvaluateRelin benchmarks are therefore out-of-place. Also avoids using plaintext/ciphertext references, to make benchmarks independent of each other.

Interestingly, this change the performance significantly on some benchmarks. I compiled SEAL with and without HEXL on ICX with clang-10.

E.g., with HEXL=OFF, I'm seeing ~1.4x speedup on CKKS EncodeDouble/DecodeDouble.

With HEXL=ON, I'm seeing significant speedups in CKKS EvaluateMulCt, e.g. on N=8192, (~1.7x), EvaluteMutPt (~2.4x) and EvaluateSquare (~2.1x) speedups compared to before this PR.

I'm not sure why the difference, perhaps reducing the number of cache misses. Anyhow, both PALISADE, Lattigo, and HElib all seem to use the same approach as this PR.

See attached for more details: seal-bench-remove-random.xlsx

@WeiDaiWD
Copy link
Contributor

WeiDaiWD commented Aug 27, 2021

There could be variance in execution time. We can guarantee a constant flow, meaning no dependency on secret data or just data, but cannot guarantee a constant time, since processor and system interrupts behaves mysteriously.

When the inputs are fixed in the current SEAL, they could fall on the faster or slower side. Then randomizing the inputs can cause big changes. I don't see how this PR will reduce cache misses though, the same values were used repeated before.

@WeiDaiWD
Copy link
Contributor

Why did you change xxx_inplace to xxx? We can also keep both.

@fboemer
Copy link
Contributor Author

fboemer commented Sep 7, 2021

Thanks for the tip; I'll close this PR until I have time to investigate further.

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