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

benchmark: remove arrays benchmark #21831

Closed
wants to merge 1 commit into from

Conversation

psmarshall
Copy link
Contributor

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

I'd like to suggest removing these benchmarks - here's a few reasons why:

  1. These benchmarks have been around for a long time. I think their original purpose was to measure performance of these typed arrays before they were in V8 (possibly, they were implemented in Node itself at some point?). V8 itself has benchmarks which cover the performance of typed arrays which we actively monitor.
  2. These benchmarks don't exercise Node itself as far as I can tell, they are essentially a benchmark of the JS engine being used. That could be a potentially useful thing to have, but I'd argue that those benchmarks would be better suited living outside of Node, or at least separately to the main actual 'Node' benchmarks.
  3. This benchmark also tests Buffer, which is interesting to benchmark from a Node point of view, but has it's own benchmark suite under benchmarks/buffer, so there is no need to duplicate it here.

@nodejs-github-bot nodejs-github-bot added benchmark Issues and PRs related to the benchmark subsystem. test Issues and PRs related to the tests. v8 engine Issues and PRs related to the V8 dependency. labels Jul 16, 2018
@psmarshall
Copy link
Contributor Author

@BridgeAR

Copy link
Member

@apapirovski apapirovski left a comment

Choose a reason for hiding this comment

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

I don't think there's much point to these anymore.

@Trott
Copy link
Member

Trott commented Jul 16, 2018

Trott pushed a commit to Trott/io.js that referenced this pull request Jul 18, 2018
PR-URL: nodejs#21831
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yang Guo <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@Trott
Copy link
Member

Trott commented Jul 18, 2018

Landed in 6dbaea7

@Trott Trott closed this Jul 18, 2018
targos pushed a commit that referenced this pull request Jul 19, 2018
PR-URL: #21831
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Yang Guo <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@targos targos mentioned this pull request Jul 31, 2018
kkty added a commit to kkty/node that referenced this pull request Jan 6, 2019
Benchmark for arrays no longer exists, but it was still referenced in
documentation.

Refs: nodejs#21831
@kkty kkty mentioned this pull request Jan 6, 2019
3 tasks
addaleax pushed a commit that referenced this pull request Jan 9, 2019
Benchmark for arrays no longer exists, but it was still referenced in
documentation.

Refs: #21831

PR-URL: #25367
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
addaleax pushed a commit that referenced this pull request Jan 9, 2019
Benchmark for arrays no longer exists, but it was still referenced in
documentation.

Refs: #21831

PR-URL: #25367
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
Benchmark for arrays no longer exists, but it was still referenced in
documentation.

Refs: nodejs#21831

PR-URL: nodejs#25367
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
BethGriggs pushed a commit that referenced this pull request Apr 28, 2019
Benchmark for arrays no longer exists, but it was still referenced in
documentation.

Refs: #21831

PR-URL: #25367
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
BethGriggs pushed a commit that referenced this pull request May 10, 2019
Benchmark for arrays no longer exists, but it was still referenced in
documentation.

Refs: #21831

PR-URL: #25367
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 16, 2019
Benchmark for arrays no longer exists, but it was still referenced in
documentation.

Refs: #21831

PR-URL: #25367
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmark Issues and PRs related to the benchmark subsystem. test Issues and PRs related to the tests. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants