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

Cleaner HEXL NTT integration #349

Merged
merged 1 commit into from
Jun 8, 2021

Conversation

fboemer
Copy link
Contributor

@fboemer fboemer commented Jun 7, 2021

  • Avoids a slowdown on first-time NTT computation due to computing NTT objects upon first use (fixes HEXL slowing down some operations in Microsoft SEAL benchmarks intel/hexl#7, see also Initial Intel HEXL integration #312 (comment))

    • This helps reduce confusion when using a small number of iterations for benchmarking, as in SEAL's examples/benchmark code. E.g. running example 7 / option 1 (BFV with default degrees) yields
      • Before this PR: (N=8192) Average batch: ~148 microseconds (on first run; faster on second run)
      • After this PR: (N=8192) Average batch: ~57 microseconds (on each run)
  • Also reduces the memory consumption: before this PR, the ntt_cache_ was stored in each compilation unit separately due to using a static variable in the header file. Running ./build/bin/sealbench yields:

    • Before this PR : [ 2590 MB] Total allocation from the memory pool
    • After this PR: [ 2388 MB] Total allocation from the memory pool
  • Fixes the HEXL commit hash, which was mistakenly pointing to the main branch instead of the 1.1.0 branch (see https://github.com/intel/hexl/tags).

@fboemer fboemer changed the title Fboemer/cleaner ntt Cleaner HEXL NTT integration Jun 7, 2021
@WeiDaiWD
Copy link
Contributor

WeiDaiWD commented Jun 8, 2021

The static fix is quite nice!

I noticed that intel_seal_ext.h and intel_seal_ext.cpp are guarded by SEAL_USE_INTEL_HEXL. In a future major release, I will add these two files to the CMake build system only if SEAL_USE_INTEL_HEXL is true. This will avoid installing "empty" headers. Similarly to ztools.h and ztools.cpp.

@WeiDaiWD WeiDaiWD merged commit a865133 into microsoft:contrib Jun 8, 2021
@fboemer fboemer deleted the fboemer/precompute-ntt branch November 3, 2021 18:24
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