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

Docs: Correct benchmark result + joseph-md5 #42

Open
cyfung1031 opened this issue Jan 9, 2024 · 0 comments · May be fixed by #43
Open

Docs: Correct benchmark result + joseph-md5 #42

cyfung1031 opened this issue Jan 9, 2024 · 0 comments · May be fixed by #43

Comments

@cyfung1031
Copy link

cyfung1031 commented Jan 9, 2024

I noticed that in your README, there is a benchmark testing, but the joseph's md5 is not included. I wonder which is faster so I tried to compare it.

Docs for joseph-md5: https://www.myersdaily.org/joseph/javascript/md5-text.html

Benchmark 1

Benchmark: https://measurethat.net/Benchmarks/Show/29171/0/md5-performance-comparison-v2

Result:

Screen Shot 2024-01-10 at 7 48 05

Benchmark 2

https://measurethat.net/Benchmarks/Show/29172/0/md5-performance-comparison-long-text-v2

Result:

Screen Shot 2024-01-10 at 7 52 30

Issue with the current benchmark in README

  1. jsperf.app is a bit unreliable on short time self execution. It should be tested with a long input.
  2. a longer string should be used to see the actual speed.
  3. One of the tests is async. deferred.resolve is required.

Correct jsperf.app with deferred.resolve + joseph-md5

Benchmark: https://jsperf.app/jonuhi/6

Screen Shot 2024-01-10 at 7 57 04

Now it comes out the result that aligns with measurethat.net.

Summary

Your library is great ! I believe it is the fastest MD5 library for sync md5 function.
I will submit a PR for updating the README :)

Note: I guess if you convert js-md5 to using low-level WebAssembly coding, it will be much faster.

@cyfung1031 cyfung1031 changed the title Docs: Add joseph-md5 to your benchmark Docs: Correct benchmark result + joseph-md5 Jan 9, 2024
@cyfung1031 cyfung1031 linked a pull request Jan 10, 2024 that will close this issue
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 a pull request may close this issue.

1 participant