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

util, assert: improve array comparison performance #22111

Closed

Conversation

BridgeAR
Copy link
Member

@BridgeAR BridgeAR commented Aug 3, 2018

See individual commits for detailed description.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

This reduces the runtime and makes sure the strict and loose options
can be tested individually.

Besides that a couple of redundant cases were removed.
This adds a smarter logic to compare object keys and it also skips
the object key comparison for arrays, if possible.

Besides that it adds a fast path for empty objects and arrays.

It also adds a fast path for boxed strings and improves the
comparison performance for TypedArrays with extra keys on the object.

On top of that a few functions are now safer to call by using
uncurryThis and by caching the actual function.
Currently the comparison could throw an error in case a boxed
primitive has no valueOf function on one side of the assert call.
@nodejs-github-bot nodejs-github-bot added the util Issues and PRs related to the built-in util module. label Aug 3, 2018
@BridgeAR BridgeAR added the performance Issues and PRs related to the performance of Node.js. label Aug 3, 2018
@BridgeAR BridgeAR added assert Issues and PRs related to the assert subsystem. benchmark Issues and PRs related to the benchmark subsystem. labels Aug 3, 2018
@jasnell
Copy link
Member

jasnell commented Aug 3, 2018

15:10:24 not ok 43 parallel/test-benchmark-assert

@BridgeAR
Copy link
Member Author

BridgeAR commented Aug 3, 2018

@jasnell I fixed the test already.

@BridgeAR
Copy link
Member Author

BridgeAR commented Aug 6, 2018

@nodejs/util @nodejs/testing PTAL

@BridgeAR
Copy link
Member Author

BridgeAR commented Aug 6, 2018

@Trott
Copy link
Member

Trott commented Aug 6, 2018

@nodejs/benchmarking

@BridgeAR
Copy link
Member Author

BridgeAR commented Aug 8, 2018

Since this PR did not yet get any attention and I have a follow-up PR. I am closing this one and just have a combined PR afterwards. See #22197

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assert Issues and PRs related to the assert subsystem. benchmark Issues and PRs related to the benchmark subsystem. performance Issues and PRs related to the performance of Node.js. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants