Skip to content

buffer: optimize createFromString - #54324

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
nxtedition:rm-createFromString
Aug 14, 2024
Merged

buffer: optimize createFromString#54324
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
nxtedition:rm-createFromString

Conversation

@ronag

@ronagronag commented Aug 12, 2024

Copy link
Copy Markdown
Member
buffers/buffer-from.js n=800000 len=100 source='array' *** -2.41 % ±0.75% ±0.99% ±1.29%
buffers/buffer-from.js n=800000 len=100 source='arraybuffer-middle' 0.17 % ±1.63% ±2.18% ±2.83%
buffers/buffer-from.js n=800000 len=100 source='arraybuffer' -0.98 % ±1.29% ±1.72% ±2.24%
buffers/buffer-from.js n=800000 len=100 source='buffer' *** -1.37 % ±0.72% ±0.95% ±1.24%
buffers/buffer-from.js n=800000 len=100 source='object' -0.60 % ±1.23% ±1.64% ±2.13%
buffers/buffer-from.js n=800000 len=100 source='string-base64' *** 2.41 % ±0.50% ±0.66% ±0.86%
buffers/buffer-from.js n=800000 len=100 source='string-utf8' *** 49.55 % ±0.67% ±0.89% ±1.16%
buffers/buffer-from.js n=800000 len=100 source='string' *** 45.78 % ±0.75% ±1.00% ±1.30%
buffers/buffer-from.js n=800000 len=100 source='uint16array' -0.75 % ±0.75% ±1.00% ±1.30%
buffers/buffer-from.js n=800000 len=100 source='uint8array' *** -3.39 % ±0.77% ±1.03% ±1.36%
buffers/buffer-from.js n=800000 len=2048 source='array' *** -0.25 % ±0.08% ±0.11% ±0.14%
buffers/buffer-from.js n=800000 len=2048 source='arraybuffer-middle' 0.59 % ±1.76% ±2.34% ±3.05%
buffers/buffer-from.js n=800000 len=2048 source='arraybuffer' 0.51 % ±1.62% ±2.16% ±2.81%
buffers/buffer-from.js n=800000 len=2048 source='buffer' * -0.81 % ±0.71% ±0.94% ±1.23%
buffers/buffer-from.js n=800000 len=2048 source='object' -0.22 % ±0.90% ±1.20% ±1.56%
buffers/buffer-from.js n=800000 len=2048 source='string-base64' *** -1.20 % ±0.34% ±0.45% ±0.59%
buffers/buffer-from.js n=800000 len=2048 source='string-utf8' *** -1.18 % ±0.45% ±0.59% ±0.77%
buffers/buffer-from.js n=800000 len=2048 source='string' *** -2.66 % ±0.51% ±0.68% ±0.89%
buffers/buffer-from.js n=800000 len=2048 source='uint16array' *** -1.01 % ±0.52% ±0.69% ±0.91%
buffers/buffer-from.js n=800000 len=2048 source='uint8array' ** -1.23 % ±0.83% ±1.10% ±1.43%

@ronag
ronag requested a review from anonrigAugust 12, 2024 06:05
@nodejs-github-botnodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Aug 12, 2024
@ronag
ronagforce-pushed the rm-createFromString branch 2 times, most recently from 73a8b66 to c37f14aCompareAugust 12, 2024 06:07
@ronag

Copy link
Copy Markdown
MemberAuthor

This will get faster once #54311 lands

@codecov

codecovBot commented Aug 12, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 89.65517% with 3 lines in your changes missing coverage. Please review.

Project coverage is 87.08%. Comparing base (298ff4f) to head (bfbfd89).
Report is 32 commits behind head on main.

FilesPatch %Lines
lib/buffer.js89.65%2 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #54324 +/- ##
==========================================
- Coverage 87.09% 87.08% -0.02% 
==========================================
Files 647 647 Lines 181816 181926 +110 Branches 34884 34898 +14 ==========================================
+ Hits 158360 158428 +68 - Misses 16764 16796 +32 - Partials 6692 6702 +10 
FilesCoverage Δ
src/node_buffer.cc71.75% <ø> (-3.44%)⬇️
lib/buffer.js96.67% <89.65%> (+0.25%)⬆️

... and 45 files with indirect coverage changes

@ronag
ronagforce-pushed the rm-createFromString branch 4 times, most recently from f36b3f2 to 19a3db1CompareAugust 12, 2024 07:51
@ronagronag changed the title buffer: rm createFromString and use fast Buffer.writebuffer: optimize createFromStringAug 12, 2024
@ronag
ronagforce-pushed the rm-createFromString branch from 19a3db1 to efd85edCompareAugust 12, 2024 07:57
@ronag
ronagforce-pushed the rm-createFromString branch from efd85ed to 59263f4CompareAugust 12, 2024 07:59
@ronag
ronagforce-pushed the rm-createFromString branch from 59263f4 to 6ae788bCompareAugust 12, 2024 07:59
@ronag
ronagforce-pushed the rm-createFromString branch from 6ae788b to 50ae2ceCompareAugust 12, 2024 08:00
@ronag
ronagforce-pushed the rm-createFromString branch from 50ae2ce to 1f9908dCompareAugust 12, 2024 08:01
@ronag
ronagforce-pushed the rm-createFromString branch 2 times, most recently from 9b240c1 to ef035e4CompareAugust 12, 2024 08:02
@ronag
ronagforce-pushed the rm-createFromString branch from 61d89e5 to df1f1c9CompareAugust 12, 2024 11:04
@ronag

Copy link
Copy Markdown
MemberAuthor

@ronag
ronagforce-pushed the rm-createFromString branch from df1f1c9 to c0a28b8CompareAugust 12, 2024 11:33
@ronag
ronagforce-pushed the rm-createFromString branch from c0a28b8 to ba4224cCompareAugust 12, 2024 11:34
@ronag
ronagforce-pushed the rm-createFromString branch from ba4224c to ba18f03CompareAugust 12, 2024 11:34
@ronag
ronagforce-pushed the rm-createFromString branch from ba18f03 to 7789f27CompareAugust 12, 2024 11:37
@ronag
ronagforce-pushed the rm-createFromString branch from 7789f27 to e697703CompareAugust 12, 2024 12:02
@ronag
ronagforce-pushed the rm-createFromString branch from e697703 to 4be82f6CompareAugust 12, 2024 12:03
@ronag
ronagforce-pushed the rm-createFromString branch from 4be82f6 to 7ed970bCompareAugust 12, 2024 12:04
@ronag
ronagforce-pushed the rm-createFromString branch from 7ed970b to 4021c79CompareAugust 12, 2024 12:09
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

nodejs-github-bot commented Aug 14, 2024

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/54324
✔ Done loading data for nodejs/node/pull/54324
----------------------------------- PR info ------------------------------------
Title buffer: optimize createFromString (#54324)
Author Robert Nagy <ronagy@icloud.com> (@ronag)
Branch ronag:rm-createFromString -> nodejs:main
Labels buffer, c++, notable-change, author ready, needs-ci
Commits 1
- buffer: optimize createFromString
Committers 1
- Robert Nagy <ronagy@icloud.com>
PR-URL: https://github.com/nodejs/node/pull/54324
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/54324
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
--------------------------------------------------------------------------------
⚠ Commits were pushed since the last approving review:
⚠ - buffer: optimize createFromString
ℹ This PR was created on Mon, 12 Aug 2024 06:05:02 GMT
✔ Approvals: 1
✔ - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/54324#pullrequestreview-2232835571
✘ This PR needs to wait 118 more hours to land (or 0 hours if there is one more approval)
✔ Last GitHub CI successful
ℹ Last Benchmark CI on 2024-08-12T11:10:02Z: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1600/
ℹ Last Full PR CI on 2024-08-14T05:54:06Z: https://ci.nodejs.org/job/node-test-pull-request/61095/
- Querying data for job/node-test-pull-request/61095/
✔ Last Jenkins CI successful
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/10383640109

@mcollinamcollina 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

Landed in 9f08320

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.bufferIssues and PRs related to the buffer subsystem.c++Issues and PRs that require attention from people who are familiar with C++.needs-ciPRs that need a full CI run.notable-changePRs with changes that should be highlighted in changelogs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@ronag@nodejs-github-bot@mcollina@anonrig