Skip to content

test: add test for effect of UV_THREADPOOL_SIZE - #49165

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
tniessen:test-uv-threadpool-size
Aug 16, 2023
Merged

test: add test for effect of UV_THREADPOOL_SIZE#49165
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
tniessen:test-uv-threadpool-size

Conversation

@tniessen

@tniessen tniessen commented Aug 14, 2023

Copy link
Copy Markdown
Member

This (not particularly elegant) native addon tests the effect of UV_THREADPOOL_SIZE on node-api. The test fails if Node.js allows more than UV_THREADPOOL_SIZE async tasks to run concurrently, or if it limits the number of concurrent async tasks to anything less than UV_THREADPOOL_SIZE.

The test schedules UV_THREADPOOL_SIZE + 1 async tasks and waits until UV_THREADPOOL_SIZE tasks are running concurrently. (If this never happens, the test times out.) All UV_THREADPOOL_SIZE tasks then wait for a second to see if the last task also starts running concurrently, which should not happen.

Admittedly, I hastily put this together and the code is sloppy. While having this test is good in any case, the main motivation is such that @anonrig can invoke the test from his own test in #48890.

@tniessen tniessen added test Issues and PRs related to Node.js core tests and test infrastructure. libuv Issues and PRs related to the libuv dependency or the uv binding. node-api Issues and PRs related to Node-API. labels Aug 14, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/gyp

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Aug 14, 2023
@tniessen tniessen mentioned this pull request Aug 14, 2023
11 tasks
@tniessen
tniessen requested a review from anonrig August 14, 2023 13:41
This (not particularly elegant) native addon tests the effect of
UV_THREADPOOL_SIZE on node-api. The test fails if Node.js allows more
than UV_THREADPOOL_SIZE async tasks to run concurrently, or if it limits
the number of concurrent async tasks to anything less than
UV_THREADPOOL_SIZE.
@tniessen
tniessen force-pushed the test-uv-threadpool-size branch from 25ac64c to c8edfb7 Compare August 14, 2023 13:43
@tniessen tniessen added the c++ Issues and PRs that require attention from people who are familiar with C++. label Aug 14, 2023
@anonrig

This comment was marked as resolved.

@anonrig anonrig left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Really good work. Thanks!

@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Aug 14, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Aug 14, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@anonrig

anonrig commented Aug 14, 2023

Copy link
Copy Markdown
Member

cc @nodejs/cpp-reviewers @nodejs/libuv

@anonrig anonrig added author ready PRs with CI started, the required approvals, and no outstanding review comments. review wanted PRs that need review. labels Aug 14, 2023

@mhdawson mhdawson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@anonrig

anonrig commented Aug 14, 2023

Copy link
Copy Markdown
Member

This works perfectly. I've tested it with the .env pull request.

@tniessen tniessen added the commit-queue PRs queued for automated landing through the Commit Queue. label Aug 15, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Aug 16, 2023
@nodejs-github-bot
nodejs-github-bot merged commit 556e95a into nodejs:main Aug 16, 2023
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 556e95a

UlisesGascon pushed a commit that referenced this pull request Sep 10, 2023
This (not particularly elegant) native addon tests the effect of
UV_THREADPOOL_SIZE on node-api. The test fails if Node.js allows more
than UV_THREADPOOL_SIZE async tasks to run concurrently, or if it limits
the number of concurrent async tasks to anything less than
UV_THREADPOOL_SIZE.

PR-URL: #49165
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
@UlisesGascon UlisesGascon mentioned this pull request Sep 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. c++ Issues and PRs that require attention from people who are familiar with C++. libuv Issues and PRs related to the libuv dependency or the uv binding. needs-ci PRs that need a full CI run. node-api Issues and PRs related to Node-API. review wanted PRs that need review. test Issues and PRs related to Node.js core tests and test infrastructure.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants