Refactor test-readline-async-iterators into a benchmark by shubham9411 · Pull Request #49237 · nodejs/node

shubham9411 · 2023-08-19T05:18:29Z

Refactor test-readline-async-iterators into a benchmark
Fixes: #49224
Added old way of readline async iterator to benchmark.

shubham9411 · 2023-08-19T05:21:34Z

Running the benchmark on my system produces following results:

readline/readline-iterable.js type="old" n=10: 18,494.459304547192
readline/readline-iterable.js type="new" n=10: 30,235.50963200886
readline/readline-iterable.js type="old" n=100: 103,827.61676284338
readline/readline-iterable.js type="new" n=100: 225,055.64552367883
readline/readline-iterable.js type="old" n=1000: 540,078.7387188078
readline/readline-iterable.js type="new" n=1000: 930,493.4352180096
readline/readline-iterable.js type="old" n=10000: 1,620,921.425907833
readline/readline-iterable.js type="new" n=10000: 2,710,950.150289306
readline/readline-iterable.js type="old" n=100000: 2,205,084.645319321
readline/readline-iterable.js type="new" n=100000: 3,754,004.952936157
readline/readline-iterable.js type="old" n=1000000: 2,301,016.7856763634
readline/readline-iterable.js type="new" n=1000000: 4,040,921.510293063

mcollina

lgtm

nodejs-github-bot · 2023-08-19T06:59:34Z

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

shubham9411 · 2023-08-19T22:04:14Z

I see two tests failed. For commit message should I change commit and push or it is ok and will be changed while merging?
For macOS test I think it is a flaky test but not sure 🤔 .

shubham9411 · 2023-08-22T17:14:59Z

Hi @mcollina, sorry you may be busy, just wanted to know if there are any changes needed from my side on this.

mertcanaltin · 2023-08-23T08:45:50Z

@shubham9411

Hello, your first commit message has an incorrect prefix; it can be formatted as follows:

test: refactor test-readline-async-iterators into a benchmark

shubham9411 · 2023-08-23T13:38:16Z

@mertcanaltin thanks, I have updated

shubham9411 · 2023-09-03T18:11:13Z

I am not sure why 3 of the tests are stuck in InProgress state. Can someone help?

mcollina · 2023-09-04T08:36:01Z

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

mcollina

lgtm

nodejs-github-bot · 2023-09-04T08:39:41Z

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

nodejs-github-bot · 2023-09-04T13:30:59Z

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

shubham9411 · 2023-09-06T16:50:21Z

Logs from test-tarball-linux suggests that all tests succeeded. But it seems to be stuck 🤔 https://github.com/nodejs/node/actions/runs/5952088308/job/16430260251

mcollina · 2023-09-21T07:56:03Z

@nodejs/build @nodejs/testing could somebody take a look at this? It keeps failing but there are less tests, not more tests, so they should not be.

nodejs-github-bot · 2023-09-21T12:47:45Z

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

nodejs-github-bot · 2023-09-21T17:12:52Z

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

richardlau · 2023-09-22T12:37:02Z

@nodejs/build @nodejs/testing could somebody take a look at this? It keeps failing but there are less tests, not more tests, so they should not be.

I started a new CI -- the only failure was a temporary infra issue while we've been adding gcc 10 to the test machines (this particular failure fixed in nodejs/build#3495). One resume CI later and the Jenkins CI has passed for this PR.

nodejs-github-bot · 2023-09-22T13:24:13Z

Commit Queue failed

- Loading data for nodejs/node/pull/49237
✔  Done loading data for nodejs/node/pull/49237
----------------------------------- PR info ------------------------------------
Title      Refactor test-readline-async-iterators into a benchmark (#49237)
Author     Shubham Pandey  (@shubham9411)
Branch     shubham9411:shubham9411/readline-iterable-benchmark -> nodejs:main
Labels     readline, test, benchmark, author ready, commit-queue-squash
Commits    2
 - test: refactor test-readline-async-iterators into a benchmark
 - empty commit
Committers 1
 - Shubham Pandey 
PR-URL: https://github.com/nodejs/node/pull/49237
Fixes: https://github.com/nodejs/node/issues/49224
Reviewed-By: Matteo Collina 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/49237
Fixes: https://github.com/nodejs/node/issues/49224
Reviewed-By: Matteo Collina 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - test: refactor test-readline-async-iterators into a benchmark
   ⚠  - empty commit
   ℹ  This PR was created on Sat, 19 Aug 2023 05:18:29 GMT
   ✔  Approvals: 1
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/49237#pullrequestreview-1609041171
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2023-09-21T17:12:52Z: https://ci.nodejs.org/job/node-test-pull-request/54135/
   ℹ  Last Benchmark CI on 2023-09-07T09:20:49Z: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1365/
- Querying data for job/node-test-pull-request/54135/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/6274851210

shubham9411 · 2023-09-23T08:00:41Z

Hey @mcollina, I pushed one empty commit just to trigger the CI pipeline. Do I have to remove it as commit queue is failing. Thanks

mcollina

lgtm

mcollina · 2023-09-23T08:10:30Z

No need

nodejs-github-bot · 2023-09-23T08:17:15Z

Landed in c0b4208

PR-URL: #49237 Fixes: #49224 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

PR-URL: nodejs#49237 Fixes: nodejs#49224 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

nodejs-github-bot added benchmark Issues and PRs related to the benchmark subsystem. needs-ci PRs that need a full CI run. readline Issues and PRs related to the built-in readline module. test Issues and PRs related to the tests. labels Aug 19, 2023

mcollina approved these changes Aug 19, 2023

View reviewed changes

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 19, 2023

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 19, 2023

github-actions bot mentioned this pull request Aug 21, 2023

CI Reliability 2023-08-21 nodejs/reliability#641

Open

35 tasks

shubham9411 mentioned this pull request Aug 21, 2023

Refactor test-readline-async-iterators into a benchmark #49224

Closed

github-actions bot mentioned this pull request Aug 22, 2023

CI Reliability 2023-08-22 nodejs/reliability#642

Open

21 tasks

github-actions bot mentioned this pull request Aug 23, 2023

CI Reliability 2023-08-23 nodejs/reliability#643

Open

27 tasks

shubham9411 force-pushed the shubham9411/readline-iterable-benchmark branch from 6a7950d to 429e78d Compare August 23, 2023 13:36

github-actions bot mentioned this pull request Aug 24, 2023

CI Reliability 2023-08-24 nodejs/reliability#644

Open

27 tasks

shubham9411 requested a review from mcollina September 1, 2023 16:47

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 4, 2023

mcollina approved these changes Sep 4, 2023

View reviewed changes

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 4, 2023

mcollina added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 4, 2023

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 7, 2023

github-actions bot mentioned this pull request Sep 21, 2023

CI Reliability 2023-09-21 nodejs/reliability#672

Open

22 tasks

richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 21, 2023

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 21, 2023

github-actions bot mentioned this pull request Sep 22, 2023

CI Reliability 2023-09-22 nodejs/reliability#673

Open

17 tasks

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 22, 2023

nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Sep 22, 2023

github-actions bot mentioned this pull request Sep 23, 2023

CI Reliability 2023-09-23 nodejs/reliability#674

Open

16 tasks

mcollina removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Sep 23, 2023

mcollina approved these changes Sep 23, 2023

View reviewed changes

mcollina added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 23, 2023

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 23, 2023

nodejs-github-bot merged commit c0b4208 into nodejs:main Sep 23, 2023

github-actions bot mentioned this pull request Sep 24, 2023

CI Reliability 2023-09-24 nodejs/reliability#675

Open

20 tasks

ruyadorno pushed a commit that referenced this pull request Sep 28, 2023

test: refactor test-readline-async-iterators into a benchmark

3db9b40

PR-URL: #49237 Fixes: #49224 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

This was referenced Sep 28, 2023

v20.7.1 proposal #49917

Closed

v20.8.0 proposal #49932

Merged

alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023

test: refactor test-readline-async-iterators into a benchmark

8ba50b8

PR-URL: nodejs#49237 Fixes: nodejs#49224 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Uh oh!

Conversation

shubham9411 commented Aug 19, 2023

Uh oh!

shubham9411 commented Aug 19, 2023

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Aug 19, 2023

Uh oh!

shubham9411 commented Aug 19, 2023

Uh oh!

shubham9411 commented Aug 22, 2023

Uh oh!

mertcanaltin commented Aug 23, 2023

Uh oh!

shubham9411 commented Aug 23, 2023

Uh oh!

shubham9411 commented Sep 3, 2023

Uh oh!

mcollina commented Sep 4, 2023

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Sep 4, 2023

Uh oh!

nodejs-github-bot commented Sep 4, 2023

Uh oh!

shubham9411 commented Sep 6, 2023

Uh oh!

mcollina commented Sep 21, 2023

Uh oh!

nodejs-github-bot commented Sep 21, 2023

Uh oh!

nodejs-github-bot commented Sep 21, 2023

Uh oh!

richardlau commented Sep 22, 2023

Uh oh!

nodejs-github-bot commented Sep 22, 2023

Uh oh!

shubham9411 commented Sep 23, 2023

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

mcollina commented Sep 23, 2023

Uh oh!

nodejs-github-bot commented Sep 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants