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

Jump to bottom

lib: refactor to use primordials in internal/priority_queue.js #36560

Closed
wants to merge 1 commit into from
Closed

lib: refactor to use primordials in internal/priority_queue.js #36560

wants to merge 1 commit into from

Conversation

Lxxyx
Copy link
Member

@Lxxyx Lxxyx commented Dec 18, 2020

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Dec 18, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 18, 2020
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@Trott
Copy link
Member

Trott commented Dec 23, 2020

Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/793/

Trott
Trott approved these changes Dec 23, 2020
View reviewed changes
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM assuming no significant performance degradation in the benchmark

@Lxxyx
Copy link
Member Author

Lxxyx commented Dec 23, 2020 •
edited

Benchmark Result:

                                                                                               confidence improvement accuracy (*)   (**)   (***)
util/format.js type='many-%' n=100000                                                                         -3.18 %       ±3.82% ±5.09%  ±6.62%
util/format.js type='no-replace-2' n=100000                                                                    5.48 %       ±7.15% ±9.51% ±12.38%
util/format.js type='no-replace' n=100000                                                                      0.75 %       ±6.68% ±8.88% ±11.56%
util/format.js type='number' n=100000                                                                         -4.01 %       ±6.25% ±8.32% ±10.85%
util/format.js type='object-%s' n=100000                                                                       1.34 %       ±1.91% ±2.54%  ±3.30%
util/format.js type='object-to-string' n=100000                                                               -0.99 %       ±5.39% ±7.18%  ±9.36%
util/format.js type='only-objects' n=100000                                                                   -0.68 %       ±1.39% ±1.85%  ±2.41%
util/format.js type='replace-object' n=100000                                                                  4.05 %       ±4.89% ±6.51%  ±8.48%
util/format.js type='string-2' n=100000                                                                        2.23 %       ±5.57% ±7.41%  ±9.65%
util/format.js type='string' n=100000                                                                 ***     -8.28 %       ±4.54% ±6.04%  ±7.86%
util/format.js type='unknown' n=100000                                                                         1.46 %       ±5.48% ±7.30%  ±9.51%
util/inspect-array.js type='denseArray' len=100000 n=500                                                      -2.32 %       ±4.91% ±6.54%  ±8.53%
util/inspect-array.js type='denseArray' len=100 n=500                                                         -4.41 %       ±5.06% ±6.73%  ±8.76%
util/inspect-array.js type='denseArray_showHidden' len=100000 n=500                                           -0.88 %       ±4.29% ±5.71%  ±7.43%
util/inspect-array.js type='denseArray_showHidden' len=100 n=500                                              -2.71 %       ±4.77% ±6.35%  ±8.27%
util/inspect-array.js type='mixedArray' len=100000 n=500                                               **      3.10 %       ±2.10% ±2.81%  ±3.67%
util/inspect-array.js type='mixedArray' len=100 n=500                                                         -0.25 %       ±4.55% ±6.06%  ±7.90%
util/inspect-array.js type='sparseArray' len=100000 n=500                                                     -1.92 %       ±3.46% ±4.61%  ±6.00%
util/inspect-array.js type='sparseArray' len=100 n=500                                                 **     -7.04 %       ±4.24% ±5.68%  ±7.45%
util/inspect.js option='colors' method='Array' n=20000                                                         2.01 %       ±2.45% ±3.26%  ±4.25%
util/inspect.js option='colors' method='Date' n=20000                                                         -1.90 %       ±4.37% ±5.82%  ±7.59%
util/inspect.js option='colors' method='Error' n=20000                                                         2.42 %       ±3.46% ±4.60%  ±5.99%
util/inspect.js option='colors' method='Number' n=20000                                                        0.09 %       ±3.94% ±5.25%  ±6.85%
util/inspect.js option='colors' method='Object_deep_ln' n=20000                                                1.74 %       ±1.83% ±2.43%  ±3.16%
util/inspect.js option='colors' method='Object_empty' n=20000                                                 -0.87 %       ±3.55% ±4.73%  ±6.16%
util/inspect.js option='colors' method='Object' n=20000                                                       -0.76 %       ±2.93% ±3.89%  ±5.07%
util/inspect.js option='colors' method='Set' n=20000                                                          -0.79 %       ±4.09% ±5.44%  ±7.09%
util/inspect.js option='colors' method='String_boxed' n=20000                                                 -1.41 %       ±4.07% ±5.41%  ±7.05%
util/inspect.js option='colors' method='String_complex' n=20000                                               -0.03 %       ±3.43% ±4.56%  ±5.94%
util/inspect.js option='colors' method='String' n=20000                                                       -1.37 %       ±3.88% ±5.16%  ±6.72%
util/inspect.js option='colors' method='TypedArray_extra' n=20000                                              0.92 %       ±3.04% ±4.07%  ±5.35%
util/inspect.js option='colors' method='TypedArray' n=20000                                                    1.56 %       ±2.07% ±2.76%  ±3.59%
util/inspect.js option='none' method='Array' n=20000                                                           2.85 %       ±4.02% ±5.40%  ±7.12%
util/inspect.js option='none' method='Date' n=20000                                                            3.71 %       ±4.58% ±6.10%  ±7.95%
util/inspect.js option='none' method='Error' n=20000                                                    *      5.38 %       ±5.35% ±7.16%  ±9.40%
util/inspect.js option='none' method='Number' n=20000                                                          0.81 %       ±3.80% ±5.08%  ±6.65%
util/inspect.js option='none' method='Object_deep_ln' n=20000                                                  0.17 %       ±2.87% ±3.82%  ±4.97%
util/inspect.js option='none' method='Object_empty' n=20000                                                   -1.45 %       ±6.22% ±8.27% ±10.76%
util/inspect.js option='none' method='Object' n=20000                                                         -1.91 %       ±3.54% ±4.73%  ±6.17%
util/inspect.js option='none' method='Set' n=20000                                                             0.31 %       ±4.28% ±5.70%  ±7.42%
util/inspect.js option='none' method='String_boxed' n=20000                                                   -1.95 %       ±4.40% ±5.85%  ±7.62%
util/inspect.js option='none' method='String_complex' n=20000                                                 -0.17 %       ±4.48% ±5.97%  ±7.78%
util/inspect.js option='none' method='String' n=20000                                                          0.78 %       ±3.74% ±4.99%  ±6.53%
util/inspect.js option='none' method='TypedArray_extra' n=20000                                               -1.13 %       ±2.68% ±3.57%  ±4.65%
util/inspect.js option='none' method='TypedArray' n=20000                                                      0.73 %       ±4.16% ±5.58%  ±7.34%
util/inspect.js option='showHidden' method='Array' n=20000                                                     0.89 %       ±1.19% ±1.58%  ±2.06%
util/inspect.js option='showHidden' method='Date' n=20000                                                     -2.91 %       ±4.78% ±6.35%  ±8.27%
util/inspect.js option='showHidden' method='Error' n=20000                                                     1.74 %       ±3.40% ±4.52%  ±5.89%
util/inspect.js option='showHidden' method='Number' n=20000                                                    0.44 %       ±2.97% ±3.96%  ±5.17%
util/inspect.js option='showHidden' method='Object_deep_ln' n=20000                                            0.32 %       ±3.20% ±4.25%  ±5.54%
util/inspect.js option='showHidden' method='Object_empty' n=20000                                             -2.01 %       ±4.34% ±5.77%  ±7.51%
util/inspect.js option='showHidden' method='Object' n=20000                                                    2.22 %       ±4.41% ±5.88%  ±7.67%
util/inspect.js option='showHidden' method='Set' n=20000                                                      -2.55 %       ±4.90% ±6.53%  ±8.52%
util/inspect.js option='showHidden' method='String_boxed' n=20000                                              1.14 %       ±4.24% ±5.65%  ±7.35%
util/inspect.js option='showHidden' method='String_complex' n=20000                                            1.57 %       ±3.86% ±5.13%  ±6.68%
util/inspect.js option='showHidden' method='String' n=20000                                                   -1.31 %       ±3.65% ±4.87%  ±6.37%
util/inspect.js option='showHidden' method='TypedArray_extra' n=20000                                          1.09 %       ±2.71% ±3.61%  ±4.70%
util/inspect.js option='showHidden' method='TypedArray' n=20000                                                1.82 %       ±2.26% ±3.01%  ±3.93%
util/inspect-proxy.js isProxy=0 showProxy=0 n=100000                                                           0.37 %       ±1.63% ±2.16%  ±2.82%
util/inspect-proxy.js isProxy=0 showProxy=1 n=100000                                                           0.47 %       ±2.40% ±3.21%  ±4.20%
util/inspect-proxy.js isProxy=1 showProxy=0 n=100000                                                          -0.70 %       ±2.42% ±3.23%  ±4.22%
util/inspect-proxy.js isProxy=1 showProxy=1 n=100000                                                          -0.38 %       ±2.44% ±3.25%  ±4.23%
util/normalize-encoding.js n=100000 input=''                                                                  -1.85 %       ±6.09% ±8.11% ±10.56%
util/normalize-encoding.js n=100000 input='base64'                                                            -0.64 %       ±4.45% ±5.92%  ±7.71%
util/normalize-encoding.js n=100000 input='BASE64'                                                            -1.57 %       ±4.26% ±5.68%  ±7.39%
util/normalize-encoding.js n=100000 input='binary'                                                            -0.47 %       ±4.32% ±5.75%  ±7.49%
util/normalize-encoding.js n=100000 input='BINARY'                                                      *     -4.84 %       ±4.70% ±6.27%  ±8.19%
util/normalize-encoding.js n=100000 input='foo'                                                                0.54 %       ±3.79% ±5.04%  ±6.56%
util/normalize-encoding.js n=100000 input='group_common'                                                       0.76 %       ±5.51% ±7.34%  ±9.55%
util/normalize-encoding.js n=100000 input='group_misc'                                                         1.14 %       ±4.14% ±5.52%  ±7.18%
util/normalize-encoding.js n=100000 input='group_uncommon'                                                     2.97 %       ±4.70% ±6.28%  ±8.21%
util/normalize-encoding.js n=100000 input='group_upper'                                                        2.47 %       ±6.37% ±8.49% ±11.09%
util/normalize-encoding.js n=100000 input='hex'                                                               -3.08 %       ±4.39% ±5.84%  ±7.61%
util/normalize-encoding.js n=100000 input='HEX'                                                                0.75 %       ±4.94% ±6.57%  ±8.55%
util/normalize-encoding.js n=100000 input='latin1'                                                            -1.06 %       ±5.31% ±7.07%  ±9.21%
util/normalize-encoding.js n=100000 input='ucs2'                                                               2.51 %       ±5.10% ±6.80%  ±8.88%
util/normalize-encoding.js n=100000 input='UCS2'                                                              -0.77 %       ±5.35% ±7.12%  ±9.26%
util/normalize-encoding.js n=100000 input='undefined'                                                         -2.94 %       ±4.62% ±6.15%  ±8.01%
util/normalize-encoding.js n=100000 input='utf16le'                                                           -0.55 %       ±6.35% ±8.45% ±11.00%
util/normalize-encoding.js n=100000 input='UTF16LE'                                                           -2.27 %       ±5.59% ±7.46%  ±9.75%
util/normalize-encoding.js n=100000 input='utf-8'                                                             -1.81 %       ±6.45% ±8.61% ±11.28%
util/normalize-encoding.js n=100000 input='utf8'                                                        *     -6.37 %       ±5.90% ±7.88% ±10.33%
util/normalize-encoding.js n=100000 input='Utf8'                                                               0.91 %       ±5.17% ±6.88%  ±8.96%
util/normalize-encoding.js n=100000 input='UTF-8'                                                             -3.67 %       ±6.27% ±8.39% ±11.03%
util/normalize-encoding.js n=100000 input='UTF8'                                                              -1.05 %       ±5.27% ±7.01%  ±9.13%
util/priority-queue.js n=100000                                                                               -4.25 %       ±4.79% ±6.39%  ±8.33%
util/splice-one.js size=100 pos='end' n=100000                                                                -3.72 %       ±5.82% ±7.75% ±10.11%
util/splice-one.js size=100 pos='middle' n=100000                                                              0.18 %       ±4.43% ±5.89%  ±7.66%
util/splice-one.js size=100 pos='start' n=100000                                                        *      4.56 %       ±3.84% ±5.11%  ±6.66%
util/splice-one.js size=10 pos='end' n=100000                                                                  2.02 %       ±6.01% ±8.01% ±10.43%
util/splice-one.js size=10 pos='middle' n=100000                                                               1.37 %       ±4.72% ±6.31%  ±8.29%
util/splice-one.js size=10 pos='start' n=100000                                                         *     -4.63 %       ±4.02% ±5.35%  ±6.97%
util/splice-one.js size=500 pos='end' n=100000                                                                 1.46 %       ±5.67% ±7.55%  ±9.83%
util/splice-one.js size=500 pos='middle' n=100000                                                              0.43 %       ±5.01% ±6.68%  ±8.70%
util/splice-one.js size=500 pos='start' n=100000                                                              -0.96 %       ±2.83% ±3.77%  ±4.91%
util/type-check.js n=100000 argument='false-object' version='js' type='ArrayBufferView'                       -1.14 %       ±6.09% ±8.12% ±10.58%
util/type-check.js n=100000 argument='false-object' version='js' type='TypedArray'                             0.08 %       ±4.33% ±5.76%  ±7.50%
util/type-check.js n=100000 argument='false-object' version='js' type='Uint8Array'                            -2.86 %       ±7.35% ±9.78% ±12.75%
util/type-check.js n=100000 argument='false-object' version='native' type='ArrayBufferView'                   -1.49 %       ±6.04% ±8.04% ±10.48%
util/type-check.js n=100000 argument='false-object' version='native' type='TypedArray'                         2.87 %       ±4.96% ±6.60%  ±8.60%
util/type-check.js n=100000 argument='false-object' version='native' type='Uint8Array'                        -0.63 %       ±5.27% ±7.02%  ±9.13%
util/type-check.js n=100000 argument='false-primitive' version='js' type='ArrayBufferView'                     1.62 %       ±6.66% ±8.86% ±11.54%
util/type-check.js n=100000 argument='false-primitive' version='js' type='TypedArray'                         -0.54 %       ±5.46% ±7.27%  ±9.46%
util/type-check.js n=100000 argument='false-primitive' version='js' type='Uint8Array'                         -2.26 %       ±5.74% ±7.65%  ±9.96%
util/type-check.js n=100000 argument='false-primitive' version='native' type='ArrayBufferView'                 4.08 %       ±6.50% ±8.66% ±11.31%
util/type-check.js n=100000 argument='false-primitive' version='native' type='TypedArray'                      1.60 %       ±5.96% ±7.95% ±10.38%
util/type-check.js n=100000 argument='false-primitive' version='native' type='Uint8Array'                     -4.16 %       ±4.38% ±5.83%  ±7.59%
util/type-check.js n=100000 argument='true' version='js' type='ArrayBufferView'                               -1.00 %       ±5.22% ±6.94%  ±9.04%
util/type-check.js n=100000 argument='true' version='js' type='TypedArray'                                     1.96 %       ±4.43% ±5.90%  ±7.68%
util/type-check.js n=100000 argument='true' version='js' type='Uint8Array'                                    -3.59 %       ±4.04% ±5.38%  ±7.03%
util/type-check.js n=100000 argument='true' version='native' type='ArrayBufferView'                           -0.38 %       ±4.64% ±6.18%  ±8.05%
util/type-check.js n=100000 argument='true' version='native' type='TypedArray'                                 0.81 %       ±5.44% ±7.24%  ±9.44%
util/type-check.js n=100000 argument='true' version='native' type='Uint8Array'                                -1.13 %       ±5.66% ±7.55%  ±9.85%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case there are 113 comparisons, you can thus
expect the following amount of false-positive results:
 5.65 false positives, when considering a   5% risk acceptance (*, **, ***),
 1.13 false positives, when considering a   1% risk acceptance (**, ***),
 0.11 false positives, when considering a 0.1% risk acceptance (***)

@aduh95
Copy link
Contributor

aduh95 commented Dec 25, 2020

Another benchmark CI to see if there's consistent regression: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/802/ (queued)

aduh95
aduh95 approved these changes Dec 25, 2020
View reviewed changes
Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No significant perf regressions, still LGTM

@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 25, 2020
@github-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 25, 2020
@github-actions
Copy link
Contributor

Landed in 5d28edc...7303afb

@github-actions github-actions bot closed this Dec 25, 2020
nodejs-github-bot pushed a commit that referenced this pull request Dec 25, 2020
@Lxxyx @nodejs-github-bot
lib: refactor to use primordials in internal/priority_queue.js
7303afb 
PR-URL: #36560
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Pooja D P <Pooja.D.P@ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@0501814314

This comment has been minimized.

Sign in to view
danielleadams pushed a commit that referenced this pull request Jan 12, 2021
@Lxxyx @danielleadams
lib: refactor to use primordials in internal/priority_queue.js
ec3e841 
PR-URL: #36560
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Pooja D P <Pooja.D.P@ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@danielleadams danielleadams mentioned this pull request Jan 12, 2021
Merged
targos pushed a commit that referenced this pull request May 25, 2021
@Lxxyx @targos
lib: refactor to use primordials in internal/priority_queue.js
6494bfc 
PR-URL: #36560
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Pooja D P <Pooja.D.P@ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos pushed a commit that referenced this pull request Jun 5, 2021
@Lxxyx @targos
lib: refactor to use primordials in internal/priority_queue.js
3c11339 
PR-URL: #36560
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Pooja D P <Pooja.D.P@ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@targos targos mentioned this pull request Jun 6, 2021
Merged
targos pushed a commit that referenced this pull request Jun 11, 2021
@Lxxyx @targos
lib: refactor to use primordials in internal/priority_queue.js
e6c0877 
PR-URL: #36560
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Pooja D P <Pooja.D.P@ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Trott Trott Trott approved these changes

@aduh95 aduh95 aduh95 approved these changes

@PoojaDurgad PoojaDurgad PoojaDurgad approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@Lxxyx @nodejs-github-bot @Trott @aduh95 @0501814314 @PoojaDurgad