crate-crypto / go-eth-kzg

Apache License 2.0
29 stars 22 forks source link

Avoid passing large arrays on heap, also use less memory #63

Closed holiman closed 6 months ago

holiman commented 7 months ago

This PR does a couple of things.

First of all, it changes the public interface methods that currently pass Blob by value, to instead pass *Blob, that is , by reference.

NOTE: This means that this PR creates a breaking change, which requires a major version-upgrade of this library, strictly speaking. :warning:

The Blob type is an array, of 130K bytes. As opposed to a slice of similar size, when an array is passed by value, every byte is passed over the heap.

Secondly, this PR pushes this change a bit further into the library, and also spot-fixes some places where memory usage can be reduced.

This PR on it's own doesn't really make a lot of difference. However, when an external user, e.g. geth, calls this library, the heap-copying might make more of a difference, since inlining cannot be performed.

Anyway, these are the differences I get with the existing benchmarks.

name                                     old time/op    new time/op    delta
/BlobToKZGCommitment-8                     24.8ms ± 3%    24.5ms ± 2%     ~     (p=0.730 n=5+4)
/ComputeKZGProof-8                         30.9ms ±11%    27.9ms ± 2%     ~     (p=0.095 n=5+5)
/ComputeBlobKZGProof-8                     32.6ms ± 2%    33.1ms ±11%     ~     (p=0.690 n=5+5)
/VerifyKZGProof-8                          3.35ms ±45%    2.51ms ±17%     ~     (p=0.548 n=5+5)
/VerifyBlobKZGProof-8                      5.12ms ±32%    5.45ms ±25%     ~     (p=0.841 n=5+5)
/VerifyBlobKZGProofBatch(count=1)-8        5.67ms ±27%    5.31ms ±15%     ~     (p=0.841 n=5+5)
/VerifyBlobKZGProofBatch(count=2)-8        10.8ms ± 9%    10.7ms ± 6%     ~     (p=0.841 n=5+5)
/VerifyBlobKZGProofBatch(count=4)-8        17.4ms ± 5%    16.5ms ± 3%     ~     (p=0.056 n=5+5)
/VerifyBlobKZGProofBatch(count=8)-8        28.6ms ± 5%    26.7ms ±10%     ~     (p=0.151 n=5+5)
/VerifyBlobKZGProofBatch(count=16)-8       47.8ms ±14%    49.0ms ± 5%     ~     (p=0.690 n=5+5)
/VerifyBlobKZGProofBatch(count=32)-8       90.4ms ±15%    87.6ms ±10%     ~     (p=1.000 n=5+5)
/VerifyBlobKZGProofBatch(count=64)-8        168ms ± 7%     156ms ± 5%     ~     (p=0.056 n=5+5)
/VerifyBlobKZGProofBatchPar(count=1)-8     6.25ms ± 3%    5.65ms ±25%     ~     (p=0.310 n=5+5)
/VerifyBlobKZGProofBatchPar(count=2)-8     6.56ms ±13%    7.11ms ± 8%     ~     (p=0.151 n=5+5)
/VerifyBlobKZGProofBatchPar(count=4)-8     8.43ms ± 3%    8.14ms ± 3%     ~     (p=0.056 n=5+5)
/VerifyBlobKZGProofBatchPar(count=8)-8     11.0ms ± 6%     9.8ms ± 3%  -10.66%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatchPar(count=16)-8    18.3ms ±10%    18.0ms ± 5%     ~     (p=1.000 n=5+5)
/VerifyBlobKZGProofBatchPar(count=32)-8    27.9ms ± 2%    30.1ms ± 7%   +7.78%  (p=0.032 n=4+5)
/VerifyBlobKZGProofBatchPar(count=64)-8    54.4ms ± 5%    56.8ms ±10%     ~     (p=0.222 n=5+5)

name                                     old alloc/op   new alloc/op   delta
/BlobToKZGCommitment-8                      383kB ± 0%     383kB ± 0%     ~     (all equal)
/ComputeKZGProof-8                         1.04MB ± 0%    0.91MB ± 0%  -12.61%  (p=0.008 n=5+5)
/ComputeBlobKZGProof-8                     1.18MB ± 0%    0.91MB ± 0%  -22.94%  (p=0.008 n=5+5)
/VerifyKZGProof-8                          6.61kB ± 0%    6.61kB ± 0%     ~     (all equal)
/VerifyBlobKZGProof-8                       540kB ± 0%     400kB ± 0%  -25.80%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatch(count=1)-8         540kB ± 0%     401kB ± 0%  -25.79%  (p=0.000 n=5+4)
/VerifyBlobKZGProofBatch(count=2)-8        1.16MB ± 0%    0.88MB ± 0%  -23.94%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatch(count=4)-8        2.23MB ± 0%    1.68MB ± 0%  -24.95%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatch(count=8)-8        4.37MB ± 0%    3.26MB ± 0%  -25.48%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatch(count=16)-8       8.65MB ± 0%    6.42MB ± 0%  -25.76%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatch(count=32)-8       17.2MB ± 0%    12.8MB ± 0%  -25.90%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatch(count=64)-8       34.3MB ± 0%    25.4MB ± 0%  -25.99%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatchPar(count=1)-8      540kB ± 0%     401kB ± 0%  -25.79%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatchPar(count=2)-8     1.08MB ± 0%    0.80MB ± 0%  -25.79%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatchPar(count=4)-8     2.16MB ± 0%    1.60MB ± 0%  -25.79%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatchPar(count=8)-8     4.32MB ± 0%    3.21MB ± 0%  -25.79%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatchPar(count=16)-8    8.64MB ± 0%    6.41MB ± 0%  -25.79%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatchPar(count=32)-8    17.3MB ± 0%    12.8MB ± 0%  -25.79%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatchPar(count=64)-8    34.6MB ± 0%    25.6MB ± 0%  -25.79%  (p=0.008 n=5+5)

name                                     old allocs/op  new allocs/op  delta
/BlobToKZGCommitment-8                       74.0 ± 0%      74.0 ± 0%     ~     (all equal)
/ComputeKZGProof-8                           86.0 ± 0%      85.0 ± 0%   -1.16%  (p=0.008 n=5+5)
/ComputeBlobKZGProof-8                       94.0 ± 0%      92.0 ± 0%   -2.13%  (p=0.008 n=5+5)
/VerifyKZGProof-8                            50.0 ± 0%      50.0 ± 0%     ~     (all equal)
/VerifyBlobKZGProof-8                        60.0 ± 0%      59.0 ± 0%   -1.67%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatch(count=1)-8          62.0 ± 0%      61.0 ± 0%   -1.61%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatch(count=2)-8           496 ± 0%       494 ± 0%   -0.48%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatch(count=4)-8           551 ± 0%       546 ± 0%   -0.91%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatch(count=8)-8           656 ± 0%       647 ± 0%   -1.40%  (p=0.016 n=5+4)
/VerifyBlobKZGProofBatch(count=16)-8          842 ± 0%       822 ± 0%   -2.35%  (p=0.016 n=4+5)
/VerifyBlobKZGProofBatch(count=32)-8        1.22k ± 0%     1.18k ± 0%   -3.06%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatch(count=64)-8        1.91k ± 0%     1.84k ± 0%   -3.78%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatchPar(count=1)-8       63.0 ± 0%      62.0 ± 0%   -1.59%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatchPar(count=2)-8        128 ± 0%       126 ± 0%   -1.56%  (p=0.029 n=4+4)
/VerifyBlobKZGProofBatchPar(count=4)-8        258 ± 0%       253 ± 0%   -1.94%  (p=0.029 n=4+4)
/VerifyBlobKZGProofBatchPar(count=8)-8        515 ± 0%       506 ± 0%   -1.75%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatchPar(count=16)-8     1.02k ± 0%     1.01k ± 0%   -1.50%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatchPar(count=32)-8     2.06k ± 0%     2.02k ± 0%   -1.60%  (p=0.008 n=5+5)
/VerifyBlobKZGProofBatchPar(count=64)-8     4.12k ± 0%     4.05k ± 0%   -1.55%  (p=0.008 n=5+5)
kevaundray commented 6 months ago

Can you comment on the difference this makes in geth?

kevaundray commented 6 months ago

I've made some changes and left some comments -- can merge once you look over them -- linter itself seems to be failing which is orthogonal to this PR (I ran it locally)

holiman commented 6 months ago

can merge once you look over them

Looks good to me!