FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

util, assert: improve array comparison performance by BridgeAR · Pull Request #22111 · nodejs/node · GitHub

/ node Public

util, assert: improve array comparison performance - #22111

Closed
BridgeAR wants to merge 5 commits into
nodejs:masterfrom
BridgeAR:improve-array-comparison-performance
ConversationCommits (5)ChecksFiles changed
Closed

util, assert: improve array comparison performance#22111
BridgeAR wants to merge 5 commits into
nodejs:masterfrom
BridgeAR:improve-array-comparison-performance

Conversation

BridgeAR commented Aug 3, 2018

Copy link
Copy Markdown
Member

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

BridgeAR added 4 commits August 3, 2018 20:50
benchmark: improve assert benchmarks
c350d3e 
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.
util,assert: improve performance
211b086 
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.
util,assert: fix boxed primitives bug
98a4f90 
Currently the comparison could throw an error in case a boxed
primitive has no valueOf function on one side of the assert call.
test: improve test coverage for comparisons
a93b55c

nodejs-github-bot commented Aug 3, 2018

Copy link
Copy Markdown
Collaborator

@BridgeAR build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/417/pipeline

nodejs-github-bot added the util Issues and PRs related to the built-in util module. label Aug 3, 2018
BridgeAR added the performance Issues and PRs related to the performance of Node.js. label Aug 3, 2018

BridgeAR commented Aug 3, 2018

Copy link
Copy Markdown
Member Author

Benchmark: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/220/
CI: https://ci.nodejs.org/job/node-test-pull-request/16178/

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 commented Aug 3, 2018

Copy link
Copy Markdown
Member 
15:10:24 not ok 43 parallel/test-benchmark-assert

BridgeAR commented Aug 3, 2018

Copy link
Copy Markdown
Member Author

@jasnell I fixed the test already.

BridgeAR commented Aug 6, 2018

Copy link
Copy Markdown
Member Author

@nodejs/util @nodejs/testing PTAL

BridgeAR commented Aug 6, 2018

Copy link
Copy Markdown
Member Author

CI https://ci.nodejs.org/job/node-test-pull-request/16223/

Trott commented Aug 6, 2018

Copy link
Copy Markdown
Member

@nodejs/benchmarking

BridgeAR commented Aug 8, 2018

Copy link
Copy Markdown
Member Author

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

BridgeAR closed this Aug 8, 2018
BridgeAR mentioned this pull request Dec 20, 2019
Closed
BridgeAR deleted the improve-array-comparison-performance branch January 20, 2020 11:34
snyk-bot mentioned this pull request Apr 1, 2020
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL