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

Fix/apple silicon performance #12479

Merged
merged 4 commits into from
Jan 13, 2023
Merged

Conversation

jackryanservia
Copy link
Contributor

@jackryanservia jackryanservia commented Jan 9, 2023

Explain your changes:

  • Added detect-gpu dependency for SnarkyJS
  • Fixed Apple silicon performance issue.
  • SnarkyJS will now detect when running on Apple silicon and use the most efficient number of workers. Required because of the issue outlined here.

Explain how you tested your changes:

  • Built version that console logged all relevant information.
  • Ran on all environment/machine combinations (Except M1 Ultra and M2 (but it's obviouse they will work)).

Caveats:

  • If the page is rendered serverside, we can't detect the platform. So we default the number of workers to a conservative guess. There is probably a way to enforce that the detect-gpu code runs client-side (in most cases), but I don't have a ton of experience with front-end frameworks and didn't think this was a high priority.
  • Mobile devices aren't accounted for yet (many of them (even if they don't run Apple silicon) will suffer from the same problem (but it's easy to add support any time)).
  • Values for the number of workers are hard-coded and based on limited testing.

Checklist:

  • Modified the current draft of release notes with details on what is completed or incomplete within this project
  • Document code purpose, how to use it
    • Mention expected invariants, implicit constraints
  • Tests were added for the new behavior
    • Document test purpose, significance of failures
    • Test names should reflect their purpose
  • All tests pass (CI will check this if you didn't)
  • Serialized types are in stable-versioned modules
  • Does this close issues? List them

After merge: o1-labs/o1js#683

Copy link
Member

@mitschabaude mitschabaude left a comment

Choose a reason for hiding this comment

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

Awesome!

@mrmr1993
Copy link
Member

mrmr1993 commented Jan 9, 2023

!ci-build-me

Copy link

@ymekuria ymekuria left a comment

Choose a reason for hiding this comment

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

Thanks for adding this!

@jackryanservia
Copy link
Contributor Author

!ci-build-me

@jackryanservia jackryanservia added the ci-build-me Add this label to trigger a circle+buildkite build for this branch label Jan 13, 2023
@Trivo25
Copy link
Member

Trivo25 commented Jan 13, 2023

!ci-build-me

@mrmr1993 mrmr1993 merged commit f2fbd2f into release/2.0.0 Jan 13, 2023
@mrmr1993 mrmr1993 deleted the fix/apple-silicon-performance branch January 13, 2023 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-build-me Add this label to trigger a circle+buildkite build for this branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants