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

Perf: revisiting field extensions in std/ #1110

Merged
merged 10 commits into from
May 4, 2024
Merged

Conversation

yelhousni
Copy link
Contributor

@yelhousni yelhousni commented Apr 23, 2024

Description

Revisiting quadratic and cubic field extensions using Karatsuba, Toom-Cook-3 and Chung-Hasan (sq). Basically experiments show that Karatsuba is better but this PR just saves some (emulated) subtractions here and there. Direct extensions using Toom-Cook work nice; for example an (unoptimized) version Toom-Cook-6 saves ~2k scs per multiplication (theoretically it saves 7 Fp-mul but uses more adds/subs) . However they are removed from this PR for now because the pairing implementation should be rewritten in the direct extension (Karabina, Granger-Scott, lines...), which will be in a separate PR.

Type of change

  • New feature (non-breaking change which adds functionality)

How has this been tested?

Existing tests work.

How has this been benchmarked?

  • PLONK verifier circuit on bw6-761:

    • old: 32,145,457 scs
    • new: 32,094,676 scs
  • Pairing check circuit e(a,b)e(c,d)==1 on BN254:

    • old: 6,674,047 scs
    • new: 6,620,773 scs
  • Pairing check circuit e(a,b)e(c,d)==1 on BLS12-381:

    • old: 8,928,354 scs
    • new: 8,792,606 scs

Checklist:

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I did not modify files generated from templates
  • golangci-lint does not output errors locally
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@yelhousni yelhousni marked this pull request as draft April 23, 2024 20:19
@yelhousni yelhousni requested a review from ivokub April 23, 2024 21:02
@yelhousni yelhousni self-assigned this Apr 23, 2024
@yelhousni yelhousni modified the milestones: v0.10.0, v0.9.0 Apr 23, 2024
@yelhousni yelhousni marked this pull request as ready for review April 23, 2024 21:03
Copy link
Collaborator

@ivokub ivokub left a comment

Choose a reason for hiding this comment

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

Looks perfect. Left a small suggestion, which is not an issue now, but may lead to a hard-to-trace bug in the future when we add multiplication to Ext6.Square after creating c3

std/algebra/emulated/fields_bw6761/e6.go Show resolved Hide resolved
Copy link
Contributor Author

@yelhousni yelhousni left a comment

Choose a reason for hiding this comment

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

@ivokub I guess we can merge this to incorporate bn254 and bls12-381 optims in master and then merge master into #1126 and resolve conflicts related to bw6.

@ivokub
Copy link
Collaborator

ivokub commented May 4, 2024

@ivokub I guess we can merge this to incorporate bn254 and bls12-381 optims in master and then merge master into #1126 and resolve conflicts related to bw6.

Yep, I think it is a good approach 👍

@yelhousni yelhousni merged commit c51abfa into master May 4, 2024
7 checks passed
@yelhousni yelhousni deleted the perf/field-extensions branch May 4, 2024 04:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants