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

lib: reduce overhead of SafePromiseAllSettledReturnVoid calls #51243

Merged

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Dec 20, 2023

It was simply calling SafePromiseAllSettled, which itself calls arrayToSafePromiseIterable which wraps all promises into new SafePromise object and wraps it into a SafeArrayIterator. Since we don't care about the return value, we can take some shortcuts.

const arrayToSafePromiseIterable = (promises, mapFn) =>
new primordials.SafeArrayIterator(
ArrayPrototypeMap(
promises,
(promise, i) =>
new SafePromise((a, b) => PromisePrototypeThen(mapFn == null ? promise : mapFn(promise, i), a, b)),
),
);

It was simply calling `SafePromiseAllSettled`, which itself calls
`arrayToSafePromiseIterable` which wraps all promises into new
`SafePromise` object and wraps it into a `SafeArrayIterator`. Since we
don't care about the return value, we can take some shortcuts.
@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Dec 20, 2023
@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 21, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 21, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 21, 2023

@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 21, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 22, 2023
@nodejs-github-bot nodejs-github-bot merged commit bc16952 into nodejs:main Dec 22, 2023
32 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in bc16952

@aduh95 aduh95 deleted the SafePromiseAllSettledReturnVoid branch December 23, 2023 00:48
RafaelGSS pushed a commit that referenced this pull request Jan 2, 2024
It was simply calling `SafePromiseAllSettled`, which itself calls
`arrayToSafePromiseIterable` which wraps all promises into new
`SafePromise` object and wraps it into a `SafeArrayIterator`. Since we
don't care about the return value, we can take some shortcuts.

PR-URL: #51243
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: James M Snell <jasnell@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Jan 2, 2024
richardlau pushed a commit that referenced this pull request Mar 25, 2024
It was simply calling `SafePromiseAllSettled`, which itself calls
`arrayToSafePromiseIterable` which wraps all promises into new
`SafePromise` object and wraps it into a `SafeArrayIterator`. Since we
don't care about the return value, we can take some shortcuts.

PR-URL: #51243
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: James M Snell <jasnell@gmail.com>
@richardlau richardlau mentioned this pull request Mar 25, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants