Skip to content

Avoids unnecessary zeroing of memory - #70

Merged
Kubuxu merged 1 commit into
filecoin-project:masterfrom
akhi3030:avoid-unnecessary-zeroing-of-mem
Aug 13, 2026
Merged

Avoids unnecessary zeroing of memory#70
Kubuxu merged 1 commit into
filecoin-project:masterfrom
akhi3030:avoid-unnecessary-zeroing-of-mem

Conversation

@akhi3030

@akhi3030 akhi3030 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

I found a few places where the library is allocating memory and zeroing it only to overwrite the memory right away. The zeroing of memory is not necessary and only degrades performance.

Further, I believe that vec![...; N] would invoke calloc which goes straight to the kernel without using the configured memory allocator. This can be detrimental for some use cases.

This PR updates these places to use MaybeUninit API which avoids unnecessary initialization of memory. And now the memory allocations should be using malloc so will go to malloc.

Copilot AI lite review requested due to automatic review settings August 7, 2026 13:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR reduces unnecessary memory zero-initialization in performance-critical pairing-related paths by switching from zero-filled allocations to MaybeUninit-backed allocations, so the memory is initialized directly by BLST routines instead of being zeroed and then overwritten.

Changes:

  • Replace zeroed Vec<u64> backing storage with Box::new_uninit_slice(...) for BLST pairing/uniq contexts.
  • Use Vec::with_capacity + spare_capacity_mut + set_len to avoid default-initializing blst_fp6 line buffers before blst_precompute_lines fills them.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/pairing.rs Switch BLST context backing allocations from zeroed Vec<u64> to uninitialized boxed slices and adjust pointer casting.
src/g2.rs Avoid default-initializing the 68-element blst_fp6 precomputation buffer by writing into spare capacity and setting length after BLST fills it.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Kubuxu

Kubuxu commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Given that it increases the code's mental overhead, as it is an additional thing that can go wrong.
Are you able to support these performance claims with benchmarks?

@akhi3030

akhi3030 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

In our project, we are doing a lot of BLS verifications and calling

fn from(affine: G2Affine) -> Self {
at a high rate.

Using a benchmark and profiling it, we see that we are spending a lot of time here.

Screenshot 2026-08-07 at 7 13 05 pm

Unfortunately, I am unable to share the benchmark with you as it depends on a bunch of internal tools. Not sure what else I can provide. A microbenchmark would show that we are saving time not unnecessarily initializing the memory. Not sure how useful that would be though.

This is the more important one for us. The other call site does not appear to be a bottleneck yet so I can roll back that change.

@akhi3030

Copy link
Copy Markdown
Contributor Author

@Kubuxu: any updates on this please?

@Kubuxu

Kubuxu commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

SGTM, CI didn't run, so I started it.
I think it still goes to the proper app allocator, even if it was using calloc, but I will merge after the CI runs.

@akhi3030

Copy link
Copy Markdown
Contributor Author

Great, thank you!

@Kubuxu
Kubuxu merged commit 4cc0dcb into filecoin-project:master Aug 13, 2026
12 checks passed
@akhi3030
akhi3030 deleted the avoid-unnecessary-zeroing-of-mem branch August 18, 2026 12:58
@akhi3030

Copy link
Copy Markdown
Contributor Author

Hey @Kubuxu. Another question please. When would be doing another release of the crate? Seems like the last release was in 2023.

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.

3 participants