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.

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

Revert "util: reduce javascript call for ToUSVString" #47340

Merged
merged 1 commit into from
Apr 1, 2023
Merged

Revert "util: reduce javascript call for ToUSVString" #47340

merged 1 commit into from
Apr 1, 2023

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Mar 31, 2023 •
edited
Loading

This PR reverts #47192 due to #47328. This change causes huge performance regression. Even though we have dont-land label on the PR, it is meddling with our benchmark results.

Fixes #47328

cc @mscdex @targos

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module. labels Mar 31, 2023
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 31, 2023
@anonrig anonrig closed this Mar 31, 2023
@anonrig

This comment was marked as outdated.

Sign in to view
@anonrig anonrig reopened this Mar 31, 2023
@anonrig
Copy link
Member Author

anonrig commented Mar 31, 2023

I'm reopening this pull request, since it's breaking our ability to validate regressions.

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 31, 2023
@nodejs-github-bot
Copy link
Collaborator

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

@anonrig anonrig added the fast-track PRs that do not need to wait for 48 hours to land. label Mar 31, 2023
@github-actions
Copy link
Contributor

Fast-track has been requested by @anonrig. Please 👍 to approve.

@anonrig anonrig added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 31, 2023
@richardlau richardlau added dont-land-on-v14.x dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. labels Mar 31, 2023
This was referenced Mar 31, 2023
Closed
Merged
@nodejs-github-bot
Copy link
Collaborator

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

@anonrig
Copy link
Member Author

anonrig commented Apr 1, 2023

7 hours have passed and the tests still didn't finish...

@targos targos added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 1, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 1, 2023
@nodejs-github-bot nodejs-github-bot merged commit 4c9ec98 into nodejs:main Apr 1, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 4c9ec98

@github-actions github-actions bot mentioned this pull request Apr 2, 2023
Open
30 tasks
@github-actions github-actions bot mentioned this pull request Apr 3, 2023
Open
28 tasks
@RafaelGSS RafaelGSS mentioned this pull request Apr 3, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BridgeAR BridgeAR BridgeAR approved these changes

@jasnell jasnell jasnell approved these changes

@Qard Qard Qard approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. fast-track PRs that do not need to wait for 48 hours to land. needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

large performance regressions since f51c152
7 participants
@anonrig @nodejs-github-bot @Qard @jasnell @BridgeAR @targos @richardlau