Skip to content

benchmark swap var for let refactor in for loops#28958

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.

Sign up for GitHub

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
Closed
RamirezAlex wants to merge 6 commits into from
Closed

benchmark swap var for let refactor in for loops #28958

RamirezAlex wants to merge 6 commits into from

Conversation

@RamirezAlex
Copy link
Contributor

@RamirezAlexRamirezAlex commented Aug 4, 2019•
edited
Loading

In benchmark directories this changes for loops using var to let
when it applies for consistency. So it finishes what started on #28867

The list of files below remain using var in the nested for loops, rest which is the majority use let now for consistency.

$ grep -l 'for (var' benchmark/*/*.js benchmark/buffers/buffer-fill.js benchmark/buffers/buffer-iterate.js benchmark/buffers/buffer-swap.js benchmark/cluster/echo.js benchmark/es/foreach-bench.js benchmark/events/ee-add-remove.js benchmark/fs/read-stream-throughput.js benchmark/misc/freelist.js benchmark/module/module-loader.js benchmark/streams/readable-bigread.js benchmark/streams/readable-bigunevenread.js benchmark/streams/readable-boundaryread.js benchmark/streams/readable-readall.js benchmark/streams/readable-unevenread.js benchmark/string_decoder/string-decoder.js benchmark/util/normalize-encoding.js benchmark/worker/echo.js
  • make -j4 test (UNIX)
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added assert Issues and PRs related to the assert subsystem. benchmark Issues and PRs related to the benchmark subsystem. cluster Issues and PRs related to the cluster subsystem. crypto Issues and PRs related to the crypto subsystem. dgram Issues and PRs related to the dgram subsystem / UDP. domain Issues and PRs related to the domain subsystem. events Issues and PRs related to the events subsystem / EventEmitter. fs Issues and PRs related to the fs subsystem / file system. v8 engine Issues and PRs related to the V8 dependency. labels Aug 4, 2019
@RamirezAlexRamirezAlex changed the title Benchmark swap var for let refactor in for loopsbenchmark swap var for let refactor in for loopsAug 4, 2019
@Trott
Copy link
Member

Trott commented Aug 7, 2019

Lite CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/3824/

@Trott
Copy link
Member

Trott commented Aug 7, 2019

Benchmark CI: https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/8415/ (queued, will 404 until a worker becomes available)

@Trott
Copy link
Member

Trott commented Aug 25, 2019

@mscdex Are you 👍, 👎 , or ❓ on these changes in the benchmark files?

@RamirezAlexRamirezAlexforce-pushed the benchmark-refactor branch 3 times, most recently from c9ede31 to aa4c956CompareAugust 30, 2019 19:39
@RamirezAlex
Copy link
ContributorAuthor

RamirezAlex commented Oct 29, 2019

Hi @Trott, I have resolved some conflicts that we recently had in this PR after more benchmarks code were introduced.
Did the Benchkmark CI queue you run lasts time went OK? Should we run those benckmarks again?
Thank you.

@BridgeAR
Copy link
Member

BridgeAR commented Jan 12, 2020

This needs a rebase. @RamirezAlex sorry that it took so long for someone to look at this again! Some times it's difficult to keep an overview over everything.

@RamirezAlexRamirezAlexforce-pushed the benchmark-refactor branch 2 times, most recently from 70c8fc1 to 54b4c84CompareFebruary 2, 2020 08:39
@richardlaurichardlau mentioned this pull request Feb 2, 2020
Closed
2 tasks
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 2, 2020

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 3, 2020

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

@Trott
Copy link
Member

Trott commented Feb 3, 2020

Benchmark suite in CI: https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/11961/

@RamirezAlex
benchmark: swap var to let in timers benchmarks
f86cdf3
In benchmark timers directory this changes for loops using var to let when it applies for consistency
@RamirezAlex
benchmark: swap var for let in path benchmarks
02e9a40
In benchmark path directory this changes for loops using var to let when it applies for consistency
@RamirezAlex
benchmark: swap var for let in os benchmarks
ca40ff3
In benchmark os directory this changes for loops using var to let when it applies for consistency
@RamirezAlex
benchmark: swap var for let in querystring benchmarks
a76f1c7
In benchmark querystring directory this changes for loops using var to let when it applies for consistency
@RamirezAlex
benchmark: swap var for let in vm benchmarks
ea0ffdb
In benchmark vm directory this changes for loops using var to let when it applies for consistency
@RamirezAlex
benchmark: swap var for let in benchmarks
7b0a8c8
In benchmark directory this changes for loops using var to let when it applies for consistency
@Trott
Copy link
Member

Trott commented Feb 3, 2020

Benchmark CI: https://ci.nodejs.org/job/node-test-commit-custom-suites-freestyle/11970/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 3, 2020

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 4, 2020

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

addaleax pushed a commit that referenced this pull request Feb 13, 2020
@RamirezAlex@addaleax
benchmark: swap var for let in benchmarks
d0ed431
In benchmark directory this changes for loops using var to let when it applies for consistency PR-URL: #28958 Reviewed-By: Anna Henningsen <[email protected]>
@addaleax
Copy link
Member

addaleax commented Feb 13, 2020

Landed in d0ed431 🎉

codebytere pushed a commit that referenced this pull request Feb 17, 2020
@RamirezAlex@codebytere
benchmark: swap var for let in benchmarks
e37f510
In benchmark directory this changes for loops using var to let when it applies for consistency PR-URL: #28958 Reviewed-By: Anna Henningsen <[email protected]>
@codebyterecodebytere mentioned this pull request Feb 17, 2020
Merged
codebytere pushed a commit that referenced this pull request Mar 15, 2020
@RamirezAlex@codebytere
benchmark: swap var for let in benchmarks
8e971ab
In benchmark directory this changes for loops using var to let when it applies for consistency PR-URL: #28958 Reviewed-By: Anna Henningsen <[email protected]>
codebytere pushed a commit that referenced this pull request Mar 17, 2020
@RamirezAlex@codebytere
benchmark: swap var for let in benchmarks
6e54536
In benchmark directory this changes for loops using var to let when it applies for consistency PR-URL: #28958 Reviewed-By: Anna Henningsen <[email protected]>
@codebyterecodebytere mentioned this pull request Mar 17, 2020
Merged
codebytere pushed a commit that referenced this pull request Mar 30, 2020
@RamirezAlex@codebytere
benchmark: swap var for let in benchmarks
7173b28
In benchmark directory this changes for loops using var to let when it applies for consistency PR-URL: #28958 Reviewed-By: Anna Henningsen <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@addaleaxaddaleaxaddaleax approved these changes

@BridgeARBridgeARAwaiting requested review from BridgeAR

Assignees

No one assigned

Labels

assertIssues and PRs related to the assert subsystem.benchmarkIssues and PRs related to the benchmark subsystem.clusterIssues and PRs related to the cluster subsystem.cryptoIssues and PRs related to the crypto subsystem.dgramIssues and PRs related to the dgram subsystem / UDP.domainIssues and PRs related to the domain subsystem.eventsIssues and PRs related to the events subsystem / EventEmitter.fsIssues and PRs related to the fs subsystem / file system.v8 engineIssues and PRs related to the V8 dependency.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@RamirezAlex@Trott@BridgeAR@nodejs-github-bot@addaleax