Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
Loading

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.

# 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>
# for free to join this conversation on GitHub. Already have an account? # 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.
6 participants
@Lxxyx @nodejs-github-bot @Trott @aduh95 @0501814314 @PoojaDurgad