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

src: use simdutf utf8 to utf16 instead of icu #46471

Closed
wants to merge 3 commits into from
Closed

src: use simdutf utf8 to utf16 instead of icu #46471

wants to merge 3 commits into from

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Feb 2, 2023

Replacing one of the crucial parts with simdutf for faster performance.

@anonrig anonrig added the performance Issues and PRs related to the performance of Node.js. label Feb 2, 2023
@anonrig anonrig requested a review from addaleax February 2, 2023 17:52
@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. labels Feb 2, 2023
@anonrig anonrig added commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. and removed c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Feb 2, 2023
@anonrig
Copy link
Member Author

anonrig commented Feb 2, 2023

cc @nodejs/cpp-reviewers

@anonrig anonrig added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Feb 2, 2023
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 2, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 2, 2023
@nodejs-github-bot
Copy link
Collaborator

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

@anonrig anonrig added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 2, 2023
@nodejs-github-bot
Copy link
Collaborator

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

@anonrig anonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 3, 2023
@targos targos mentioned this pull request Feb 3, 2023
Closed
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 3, 2023
@anonrig
Copy link
Member Author

anonrig commented Feb 3, 2023

Appreciate re-reviews @targos @RafaelGSS @addaleax

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

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

@nodejs-github-bot
Copy link
Collaborator

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

@anonrig
Copy link
Member Author

anonrig commented Feb 4, 2023

Is there a bug on this pull request, or are these flaky tests?

This was referenced Feb 5, 2023
Open
Open
@nodejs-github-bot
Copy link
Collaborator

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

@anonrig anonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 6, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 6, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 6a60ed8...8a4b1c9

nodejs-github-bot pushed a commit that referenced this pull request Feb 6, 2023
@anonrig @nodejs-github-bot
src: use simdutf utf8 to utf16 instead of icu
cf60b1e 
PR-URL: #46471
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: James M Snell <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Feb 6, 2023
@anonrig @nodejs-github-bot
src: use string_view instead of std::string&
a086363 
PR-URL: #46471
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: James M Snell <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Feb 6, 2023
@anonrig @nodejs-github-bot
src: remove unused includes from main thread
8a4b1c9 
PR-URL: #46471
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@anonrig anonrig deleted the improve-utf16-conversion branch February 6, 2023 17:47
@github-actions github-actions bot mentioned this pull request Feb 7, 2023
Open
18 tasks
MylesBorins pushed a commit that referenced this pull request Feb 18, 2023
@anonrig @MylesBorins
src: use simdutf utf8 to utf16 instead of icu
f35f6d2 
PR-URL: #46471
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 18, 2023
@anonrig @MylesBorins
src: use string_view instead of std::string&
60c2a86 
PR-URL: #46471
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 18, 2023
@anonrig @MylesBorins
src: remove unused includes from main thread
f72f643 
PR-URL: #46471
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Feb 19, 2023
Merged
@StefanStojanovic StefanStojanovic mentioned this pull request Feb 22, 2023
Closed
StefanStojanovic added a commit to JaneaSystems/node that referenced this pull request Feb 22, 2023
@StefanStojanovic
win,src: revert simdutf to icu for win arm64
b7fbc9b 
Many tests started failing on ARM64 Windows after migrating from icu to
simdutf. This change reverts those changes for the problematic platform.

Refs: nodejs#46471
Refs: nodejs#46472
Refs: nodejs#46548
Refs: simdutf/simdutf#216
@StefanStojanovic StefanStojanovic mentioned this pull request Feb 22, 2023
Closed
danielleadams pushed a commit that referenced this pull request Apr 11, 2023
@anonrig @danielleadams
src: use simdutf utf8 to utf16 instead of icu
f1c7700 
PR-URL: #46471
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: James M Snell <[email protected]>
danielleadams pushed a commit that referenced this pull request Apr 11, 2023
@anonrig @danielleadams
src: use string_view instead of std::string&
b9ca302 
PR-URL: #46471
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: James M Snell <[email protected]>
danielleadams pushed a commit that referenced this pull request Apr 11, 2023
@anonrig @danielleadams
src: remove unused includes from main thread
b3b3b33 
PR-URL: #46471
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@danielleadams danielleadams mentioned this pull request Apr 11, 2023
Merged
codebytere added a commit to electron/electron that referenced this pull request Apr 15, 2023
@codebytere
fix: add simdutf dep to src/inspector BUILD.gn
5305acb 
- nodejs/node#46471
- nodejs/node#46472
codebytere added a commit to electron/electron that referenced this pull request Apr 16, 2023
@codebytere
fix: add simdutf dep to src/inspector BUILD.gn
b21ca56 
- nodejs/node#46471
- nodejs/node#46472
codebytere added a commit to electron/electron that referenced this pull request Apr 17, 2023
@codebytere
fix: add simdutf dep to src/inspector BUILD.gn
8096620 
- nodejs/node#46471
- nodejs/node#46472
codebytere added a commit to electron/electron that referenced this pull request Apr 18, 2023
@electron-roller @codebytere
chore: bump node to v18.16.0 (main) (#37973)
de192c2 
* chore: bump node in DEPS to v18.16.0

* build,test: add proper support for IBM i

nodejs/node#46739

* lib: enforce use of trailing commas

nodejs/node#46881

* src: add initial support for single executable applications

nodejs/node#45038

* lib: do not crash using workers with disabled shared array buffers

nodejs/node#41023

* src: remove shadowed variable in OptionsParser::Parse

nodejs/node#46672

* src: allow embedder control of code generation policy

nodejs/node#46368

* src: allow optional Isolate termination in node::Stop()

nodejs/node#46583

* lib: fix BroadcastChannel initialization location

nodejs/node#46864

* chore: fixup patch indices

* chore: sync filenames.json

* fix: add simdutf dep to src/inspector BUILD.gn

- nodejs/node#46471
- nodejs/node#46472

* deps: replace url parser with Ada

nodejs/node#46410

* tls: support automatic DHE

nodejs/node#46978

* fixup! src: add initial support for single executable applications

* http: unify header treatment

nodejs/node#46528

* fix: libc++ buffer overflow in string_view ctor

nodejs/node#46410

* test: include strace openat test

nodejs/node#46150

* fixup! fixup! src: add initial support for single executable applications

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@targos targos targos left review comments

@bnoordhuis bnoordhuis bnoordhuis left review comments

@RafaelGSS RafaelGSS RafaelGSS approved these changes

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell 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++. commit-queue-failed An error occurred while landing this pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. needs-ci PRs that need a full CI run. performance Issues and PRs related to the performance of Node.js.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@anonrig @nodejs-github-bot @bnoordhuis @jasnell @addaleax @targos @RafaelGSS