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

Much faster solution #2

Merged
merged 3 commits into from Jan 26, 2019
Merged

Much faster solution #2

merged 3 commits into from Jan 26, 2019

Conversation

arguiot
Copy link
Contributor

@arguiot arguiot commented Jan 26, 2019

Proposing an incredibly fast solution for checking if a number is prime or not.
Currently working fine until Number.MAX_SAFE_INTEGER.

This was inspired by the code I have written for https://github.com/arguiot/TheoremJS. I already told you about this in #1

@arguiot
Copy link
Contributor Author

arguiot commented Jan 26, 2019

Simple comparisons:

  • v2: all tests were done in 27s
  • my version: all tests were done in 14ms

And it's not a probabilistic test.

@lukeed
Copy link
Owner

lukeed commented Jan 26, 2019

This is great, thank you~! I did see your original comment in #1 – I just wanted to mess around and implement AKS since I didn't see an implementation anywhere. PS, I know it's slow 😄

Is this a known algorithm / theorem? Or it a brute force method you've come up with?

I pushed a change to this PR with a refactored version. It's 50B smaller (gzip) and is actually significantly faster, too. It's the same logic, but you were checking against (and reassigning) the out value repeatedly & unnecessarily, as you totally could have aborted the loop early.

While the primes bench is a little bit slower, I'd personally much prefer to trade a 100k loss on rare occasion for 9-14M gain the majority of the time 🎉

Here are the benchmarks of your original PR against the refactor. I've also included Theorem, if that's okay! The number sets are taken from the tests.

# Primes
theorem  x  64,608 ops/sec ±4.40% (93 runs sampled)
before   x 932,150 ops/sec ±0.50% (94 runs sampled)
after    x 853,139 ops/sec ±0.28% (97 runs sampled)

# Not Primes
theorem  x    234,204 ops/sec ±0.24% (98 runs sampled)
before   x  4,391,996 ops/sec ±0.28% (97 runs sampled)
after    x 18,300,594 ops/sec ±0.26% (97 runs sampled)

# Carmichael Numbers
theorem  x    57,118 ops/sec ±0.30% (95 runs sampled)
before   x    73,004 ops/sec ±0.40% (93 runs sampled)
after    x 9,271,695 ops/sec ±0.53% (90 runs sampled)

@lukeed lukeed merged commit 9d32d97 into lukeed:master Jan 26, 2019
@arguiot
Copy link
Contributor Author

arguiot commented Jan 26, 2019

Great! TheoremJS should be slower as it uses the BigNumber API for all the computations.

It's an algorithm I came with, but it's based on Eratosthenes' sieve.

@lukeed
Copy link
Owner

lukeed commented Jan 26, 2019

Yeah, I saw that. Totally makes sense.

Thanks! The name was escaping me, I was thinking it began with an S___ 🙃

@lukeed
Copy link
Owner

lukeed commented Jan 26, 2019

Hey @arguiot, I switched approaches again~! If you check out the latest README, you can find benchmarks of the new approach.

@arguiot
Copy link
Contributor Author

arguiot commented Jan 27, 2019

@lukeed great 👍. I saw that in arguiot.js you called me Arthur Goi, But my name is Arthur Guiot 🙃. Otherwise, I really like this method.

@lukeed
Copy link
Owner

lukeed commented Jan 27, 2019

Oh shoot, sorry! I was distracted. 🙈

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

2 participants