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

Performance of this deep equal implementation is generally proportionally between 5x slower and 700x slower compared to native deepStrictEqual #109

Open
unphased opened this issue Mar 29, 2024 · 1 comment

Comments

@unphased
Copy link

unphased commented Mar 29, 2024

I have a test implemented here using my test/benchmark library. I've also taken the plot that this generates and hosted it publicly on the net here: https://unphased.github.io/static-sharing/2024-03-29/deepequal_perf/deepequal_perf_collisions.html

I like this method of sharing benchmark results in HTML format since the plot made with uplot allows for interacting with series and zooming in.

The general summary based on the above plotting is that although this deep-equal implementation does not have an asymptotic algorithmic shortcoming compared to the builtin (as demonstrated by the straight lines in the first plot), it is consistently between 5x slower (if I compare large arrays filled with integers) and 700x (!!) slower (if I compare large arrays filled with objects). Obviously being implemented in js as opposed to presumably C++ will necessitate a 2-10x performance hit, something seems suboptimal here because this is a lot slower than that.

I first became aware of the issue being in this library by seeing it show up in the chrome profiler. I haven't reviewed the code yet, but based on the flamegraph that I saw, the implementation appears to be a recursive one. And that doesn't bode well for performance, but I would expect even a recursive implementation to be faster than what I'm seeing.

Please let me know if any of this is unclear.

@ljharb
Copy link
Member

ljharb commented Mar 29, 2024

If there's a way to maintain correctness and compatibility while increasing performance, I'm all for it.

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

No branches or pull requests

2 participants