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 #683

Merged
merged 5 commits into from
Jan 13, 2023
Merged

Fix/apple silicon performance #683

merged 5 commits into from
Jan 13, 2023

Conversation

jackryanservia
Copy link
Contributor

First merge: MinaProtocol/mina#12479

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.

  • Closes Run using only one thread on M1 processors #491

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.

Excellent work!

'apple m1 max': 3,
'apple m1 ultra': 7,
'apple m2': 2,
'SSR': ((numCpus / 6) >> 0) + 1,
Copy link
Member

Choose a reason for hiding this comment

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

could you elaborate on that case? I'd assume that if server-side rendering prevents getGPUTier from running on the client, then snarkyjs won't run on the client in its entirety, so this case doesn't really matter? Or am I wrong?

Copy link
Member

@Trivo25 Trivo25 left a comment

Choose a reason for hiding this comment

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

This is great :) Would you mind adding the fix to the changelog?

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.

Run using only one thread on M1 processors
3 participants