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

feat: aggregate with randomness #139

Merged
merged 23 commits into from
May 24, 2024
Merged

Conversation

matthewkeil
Copy link
Member

@matthewkeil matthewkeil commented Apr 29, 2024

NOTE: This PR is based on code in #144, #145, #146. Need to update merge branch to master once those PR's are merged. Do not merge until that happens but is ready for review

** Motivation **

Aggregation of signatures in lodestar can take up to 25% of cpu thread time. The process includes a lot of generated JS objects that immediately get thrown away once the signatures are aggregated. This PR is designed to create a single signature call that will do the deserialization and aggregation, with added randomness, in a single function call. By moving this to native the JS objects do not need to be created so only the native deserialization is necessary. Created an async version as well to move this time-intensive work to the worker pool.

** Description of Inclusions **

  • Add aggregateWithRandomness and asyncAggregateWithRandomness functions and unit tests
  • Add perf test comparison between current implementation and aggregateWithRandomness
  • Add test helper function getTestSetsSameMessage and SameMessageTestSets type

@matthewkeil matthewkeil requested a review from a team as a code owner April 29, 2024 05:45
@matthewkeil matthewkeil force-pushed the mkeil/aggregate-with-randomness branch from 3ef1c8b to 6539f39 Compare April 29, 2024 05:51
@matthewkeil matthewkeil marked this pull request as draft April 29, 2024 05:54
@matthewkeil matthewkeil changed the title Mkeil/aggregate with randomness feat: aggregate with randomness Apr 29, 2024
@matthewkeil matthewkeil force-pushed the mkeil/aggregate-with-randomness branch from 6539f39 to 6b99da1 Compare April 29, 2024 13:21
Copy link

github-actions bot commented Apr 29, 2024

Performance Report

✔️ no performance regression detected

Full benchmark results
Benchmark suite Current: 27e7d7a Previous: - Ratio
PublicKey serialization 923.00 ns/op
PublicKey deserialize 22.550 us/op
PublicKey deserialize and validate - 1 keys 85.251 us/op
PublicKey deserialize and validate - 100 keys 8.3387 ms/op
PublicKey deserialize and validate - 10000 keys 831.12 ms/op
SecretKey.fromKeygen 2.5860 us/op
SecretKey serialization 861.00 ns/op
SecretKey deserialization 1.4180 us/op
SecretKey.toPublicKey 139.34 us/op
SecretKey.sign 530.91 us/op
Signature serialization 1.0940 us/op
Signature deserialize 44.427 us/op
Signatures deserialize and validate - 1 sets 120.46 us/op
Signatures deserialize and validate - 100 sets 12.045 ms/op
Signatures deserialize and validate - 10000 sets 1.1974 s/op
aggregatePublicKeys - 1 sets 2.4410 us/op
aggregatePublicKeys - 8 sets 10.364 us/op
aggregatePublicKeys - 32 sets 37.900 us/op
aggregatePublicKeys - 128 sets 147.73 us/op
aggregatePublicKeys - 256 sets 294.38 us/op
aggregateSignatures - 1 sets 4.1760 us/op
aggregateSignatures - 8 sets 23.720 us/op
aggregateSignatures - 32 sets 91.500 us/op
aggregateSignatures - 128 sets 360.94 us/op
aggregateSignatures - 256 sets 723.85 us/op
JS version of aggregateWithRandomness 295.57 ms/op
native version of aggregateWithRandomness 289.17 ms/op
aggregateVerify - 1 sets 1.5742 ms/op
aggregateVerify - 8 sets 5.7259 ms/op
aggregateVerify - 32 sets 20.403 ms/op
aggregateVerify - 128 sets 78.775 ms/op
aggregateVerify - 256 sets 156.67 ms/op
verifyMultipleAggregateSignatures - 1 sets 1.7510 ms/op
verifyMultipleAggregateSignatures - 8 sets 7.0457 ms/op
verifyMultipleAggregateSignatures - 32 sets 25.562 ms/op
verifyMultipleAggregateSignatures - 128 sets 99.930 ms/op
verifyMultipleAggregateSignatures - 256 sets 198.74 ms/op
Same message - 1 sets 1.7027 ms/op
Same message - 8 sets 2.5719 ms/op
Same message - 32 sets 5.5196 ms/op
Same message - 128 sets 17.749 ms/op
Same message - 256 sets 33.192 ms/op
libuv multithreading - addVerificationRandomness true 23.774 s/op
libuv multithreading - addVerificationRandomness false 23.802 s/op

by benchmarkbot/action

@matthewkeil matthewkeil force-pushed the mkeil/aggregate-with-randomness branch from 9fcfe35 to ccc1a7f Compare May 9, 2024 13:25
@matthewkeil matthewkeil changed the base branch from master to mkeil/fix-spec-tests May 9, 2024 13:26
@matthewkeil matthewkeil marked this pull request as ready for review May 10, 2024 09:59
Base automatically changed from mkeil/fix-spec-tests to master May 23, 2024 10:42
@matthewkeil matthewkeil changed the base branch from master to mkeil/update-spec-tests May 23, 2024 10:50
@matthewkeil matthewkeil changed the base branch from mkeil/update-spec-tests to master May 23, 2024 10:50
@matthewkeil matthewkeil force-pushed the mkeil/aggregate-with-randomness branch from 43d1a69 to e61571b Compare May 23, 2024 13:52
@matthewkeil matthewkeil force-pushed the mkeil/aggregate-with-randomness branch from e61571b to 8f359cb Compare May 23, 2024 14:25
scripts/install.ts Outdated Show resolved Hide resolved
src/addon.h Show resolved Hide resolved
src/aggregate_with_randomness.cc Outdated Show resolved Hide resolved
src/aggregate_with_randomness.cc Outdated Show resolved Hide resolved
src/aggregate_with_randomness.cc Outdated Show resolved Hide resolved
src/aggregate_with_randomness.cc Outdated Show resolved Hide resolved
src/aggregate_with_randomness.cc Outdated Show resolved Hide resolved
@matthewkeil matthewkeil merged commit dbd9920 into master May 24, 2024
28 checks passed
@matthewkeil matthewkeil deleted the mkeil/aggregate-with-randomness branch May 24, 2024 14:57
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.

None yet

3 participants