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

build: update gmp to 6.3.0 #6317

Merged
merged 2 commits into from
Oct 8, 2024
Merged

build: update gmp to 6.3.0 #6317

merged 2 commits into from
Oct 8, 2024

Conversation

kwvg
Copy link
Collaborator

@kwvg kwvg commented Oct 5, 2024

Additional Information

After bls-signatures#92, GMP is re-enabled for Apple Silicon macOS targets so long as GMP 6.3.0 or higher is used. GMP significantly contributes to performance improvements in bls-signatures, generally to the tune of ~50% (source).

The URL has been changed based on guidance from the Homebrew recipe for gmp (source).

Breaking Changes

None expected.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas (note: N/A)
  • I have added or updated relevant unit/integration/functional/e2e tests (note: N/A)
  • I have made corresponding changes to the documentation (note: N/A)
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

@kwvg kwvg force-pushed the bump_gmp branch 2 times, most recently from 3117d56 to cc9bd9b Compare October 6, 2024 05:32
@kwvg kwvg marked this pull request as ready for review October 6, 2024 05:48
depends/packages/gmp.mk Outdated Show resolved Hide resolved
Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

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

utACK 9fc3440


define $(package)_set_vars
$(package)_config_opts+=--enable-cxx --enable-fat --disable-shared
$(package)_config_opts += --disable-shared --enable-cxx --enable-fat
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: why did you re-ordered this one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alphabetical order

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK 9fc3440

Copy link
Collaborator

@ogabrielides ogabrielides left a comment

Choose a reason for hiding this comment

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

ACK 9fc3440

Comment on lines +9 to +11
$(package)_cflags_aarch64 += -march=armv8-a
$(package)_cflags_armv7l += -march=armv7-a
$(package)_cflags_x86_64 += -march=x86-64
Copy link
Member

Choose a reason for hiding this comment

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

has any thought been had to risc-v?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's unclear what would be a suitable "base" march for 64-bit RISC-V since there seems to be limited use of riscv64 as such a parameter.

rv64i would be making the fewest possible assumptions (the E variant is supported only assembly tools by LLVM as of this writing, source, from here on out, let's assume the I variant) but on the other hand, there is documentation that asserts that rv64g is complete for general computing (source) and projects like Debian use rv64gc as the baseline (source) though vector extensions require rv64gcv.

Extensions are enumerated in the march explicitly, of which there are many (source) though LLVM offers 6 different profiles (see source above) based on the architecture profiles specified by the RISC-V foundation (source).


As an aside, the x86-64 march is a base profile for AMD64 but we could investigate if we should enforce minimum processor requirements in our AMD64 binaries if benchmarks prove provide significant performance over the base profile being used now but that's an exercise for a future PR.

@ogabrielides
Copy link
Collaborator

Benchmarks:

develop:

ns/op op/s err% total benchmark
70,411.89 14,202.15 0.1% 0.84 BLSDKG_BuildQuorumVerificationVectors_simple_10
6,963,633.40 143.60 0.1% 0.84 BLSDKG_BuildQuorumVerificationVectors_simple_100
121,542,042.00 8.23 1.8% 1.34 BLSDKG_BuildQuorumVerificationVectors_simple_400
356,840,361.00 2.80 0.2% 21.16 BLSDKG_GenerateContributions_simple_100
77,746,474.67 12.86 0.6% 46.49 BLSDKG_GenerateContributions_simple_50
35,672,763.35 28.03 0.4% 42.65 BLSDKG_InitDKG_simple_10
97,649,017.33 10.24 0.9% 11.75 BLSDKG_InitDKG_simple_50
6,160,490.84 162.32 1.0% 7.36 BLSDKG_VerifyContributionShares_aggregated_10
194,511,154.20 5.14 2.0% 23.12 BLSDKG_VerifyContributionShares_aggregated_100
1,395,868,542.00 0.72 0.4% 15.36 BLSDKG_VerifyContributionShares_aggregated_400
18,114,715.91 55.20 2.0% 2.16 BLSDKG_VerifyContributionShares_simple_10
682,616,237.50 1.46 4.4% 81.68 BLSDKG_VerifyContributionShares_simple_100
5,243,380,834.00 0.19 1.1% 56.98 BLSDKG_VerifyContributionShares_simple_400
1,278.96 781,887.91 1.8% 0.01 BLS_PubKeyAggregate_Normal
488.71 2,046,222.01 2.5% 0.01 BLS_SecKeyAggregate_Normal
1,616,572.74 618.59 0.3% 1.93 BLS_Sign_Normal
2,802.03 356,884.49 0.4% 0.01 BLS_SignatureAggregate_Normal
2,532,292.54 394.90 1.4% 30.86 BLS_Verify_Batched
962,720.30 1,038.72 0.2% 11.56 BLS_Verify_BatchedParallel
187,074,000.00 5.35 0.4% 22.43 BLS_Verify_LargeAggregatedBlock100
1,904,606,167.00 0.53 1.4% 21.03 BLS_Verify_LargeAggregatedBlock1000
190,056,050.00 5.26 0.4% 22.70 BLS_Verify_LargeAggregatedBlock1000PreVerified
367,929,322.92 2.72 0.8% 44.26 BLS_Verify_LargeBlock100
3,694,873,417.00 0.27 0.7% 40.67 BLS_Verify_LargeBlock1000
191,966,458.33 5.21 1.8% 22.97 BLS_Verify_LargeBlockSelfAggregated100
1,872,743,458.00 0.53 1.1% 43.85 BLS_Verify_LargeBlockSelfAggregated1000
3,616,298.91 276.53 0.5% 0.87 BLS_Verify_Normal

GMP bump

ns/op op/s err% total benchmark
72,606.97 13,772.78 0.6% 0.86 BLSDKG_BuildQuorumVerificationVectors_simple_10
7,124,783.30 140.36 0.9% 0.86 BLSDKG_BuildQuorumVerificationVectors_simple_100
125,689,792.00 7.96 0.2% 1.39 BLSDKG_BuildQuorumVerificationVectors_simple_400
353,911,312.50 2.83 0.3% 20.91 BLSDKG_GenerateContributions_simple_100
77,434,504.31 12.91 0.4% 46.22 BLSDKG_GenerateContributions_simple_50
35,790,309.63 27.94 0.3% 42.77 BLSDKG_InitDKG_simple_10
96,176,970.90 10.40 0.3% 11.55 BLSDKG_InitDKG_simple_50
6,027,378.44 165.91 0.3% 7.19 BLSDKG_VerifyContributionShares_aggregated_10
186,456,326.42 5.36 1.6% 22.21 BLSDKG_VerifyContributionShares_aggregated_100
1,372,290,083.00 0.73 2.8% 14.83 BLSDKG_VerifyContributionShares_aggregated_400
18,082,068.18 55.30 1.4% 2.17 BLSDKG_VerifyContributionShares_simple_10
686,483,075.00 1.46 1.8% 82.20 BLSDKG_VerifyContributionShares_simple_100
5,227,349,916.00 0.19 0.9% 57.68 BLSDKG_VerifyContributionShares_simple_400
1,256.50 795,859.11 0.1% 0.01 BLS_PubKeyAggregate_Normal
478.32 2,090,648.85 1.7% 0.01 BLS_SecKeyAggregate_Normal
1,559,276.82 641.32 0.3% 1.87 BLS_Sign_Normal
2,895.38 345,377.26 0.8% 0.01 BLS_SignatureAggregate_Normal
2,510,477.66 398.33 1.9% 29.57 BLS_Verify_Batched
950,347.90 1,052.25 0.1% 11.36 BLS_Verify_BatchedParallel
188,279,725.67 5.31 0.6% 22.69 BLS_Verify_LargeAggregatedBlock100
1,857,356,875.00 0.54 1.0% 20.47 BLS_Verify_LargeAggregatedBlock1000
186,767,302.08 5.35 0.1% 22.46 BLS_Verify_LargeAggregatedBlock1000PreVerified
363,135,787.50 2.75 0.3% 43.80 BLS_Verify_LargeBlock100
3,701,206,708.00 0.27 1.1% 40.64 BLS_Verify_LargeBlock1000
186,235,340.25 5.37 0.1% 22.38 BLS_Verify_LargeBlockSelfAggregated100
1,845,792,771.00 0.54 0.1% 40.60 BLS_Verify_LargeBlockSelfAggregated1000
3,637,575.43 274.91 0.4% 0.87 BLS_Verify_Normal
`

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK 9dad525

(no diff from prior)

@PastaPastaPasta PastaPastaPasta merged commit 4843f04 into dashpay:develop Oct 8, 2024
9 of 10 checks passed
@UdjinM6 UdjinM6 added this to the 22 milestone Oct 29, 2024
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.

5 participants