Skip to content

buffer: avoid creating unnecessary environment - #58053

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
anonrig:yagiz/avoid-environment
Apr 29, 2025
Merged

buffer: avoid creating unnecessary environment#58053
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
anonrig:yagiz/avoid-environment

Conversation

@anonrig

@anonriganonrig commented Apr 27, 2025•
edited
Loading

Copy link
Copy Markdown
Member

We don't have to deference the args pointer to create an environment variable while having access to isolate using args.GetIsolate()

@anonrig
anonrig requested review from jasnell and ronagApril 27, 2025 17:22
@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 Apr 27, 2025
@anonrig
anonrigforce-pushed the yagiz/avoid-environment branch from 523f5a8 to 4df7266CompareApril 27, 2025 17:24
juanarbol
juanarbol approved these changes Apr 27, 2025
View reviewed changes

@juanarboljuanarbol 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.

sgtm

@anonriganonrig 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 Apr 27, 2025
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 27, 2025
@nodejs-github-bot

nodejs-github-bot commented Apr 27, 2025

Copy link
Copy Markdown
Collaborator

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

jasnell
jasnell approved these changes Apr 27, 2025
View reviewed changes

@jasnelljasnell 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.

To be fair, the changed code is not creating an Environment unnecessarily, it is simply dereferencing a pointer.

@anonrig

anonrig commented Apr 27, 2025

Copy link
Copy Markdown
MemberAuthor

To be fair, the changed code is not creating an Environment unnecessarily, it is simply dereferencing a pointer.

Ah true. Thanks. Still seems like an unnecessary operation that we do in almost all places.

@codecov

codecovBot commented Apr 27, 2025•
edited
Loading

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.20%. Comparing base (e0cf8ae) to head (4df7266).
Report is 14 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #58053 +/- ##
==========================================
+ Coverage 90.18% 90.20% +0.01% 
==========================================
Files 630 630 Lines 186393 186393 Branches 36595 36594 -1 ==========================================
+ Hits 168103 168140 +37 + Misses 11090 11081 -9 + Partials 7200 7172 -28
Files with missing linesCoverage Δ
src/node_buffer.cc67.51% <100.00%> (ø)

... and 24 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@anonriganonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 27, 2025
@nodejs-github-bot

nodejs-github-bot commented Apr 27, 2025

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Apr 27, 2025

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-bot

nodejs-github-bot commented Apr 28, 2025

Copy link
Copy Markdown
Collaborator

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

@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 29, 2025
@nodejs-github-bot
nodejs-github-bot merged commit adc948a into nodejs:mainApr 29, 2025
@nodejs-github-bot

nodejs-github-bot commented Apr 29, 2025

Copy link
Copy Markdown
Collaborator

Landed in adc948a

RafaelGSS pushed a commit that referenced this pull request May 1, 2025
@anonrig@RafaelGSS
buffer: avoid creating unnecessary environment
c2150e0
PR-URL: #58053
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
@RafaelGSSRafaelGSS mentioned this pull request May 1, 2025
Merged
RafaelGSS pushed a commit that referenced this pull request May 2, 2025
@anonrig@RafaelGSS
buffer: avoid creating unnecessary environment
ca86c93
PR-URL: #58053
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
aduh95 pushed a commit that referenced this pull request May 6, 2025
@anonrig@aduh95
buffer: avoid creating unnecessary environment
f89b36e
PR-URL: #58053
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
aduh95 pushed a commit that referenced this pull request May 6, 2025
@anonrig@aduh95
buffer: avoid creating unnecessary environment
1c8e476
PR-URL: #58053
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
RafaelGSS pushed a commit that referenced this pull request May 14, 2025
@anonrig@RafaelGSS
buffer: avoid creating unnecessary environment
aa14ff3
PR-URL: #58053
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
aduh95 pushed a commit that referenced this pull request May 16, 2025
@anonrig@aduh95
buffer: avoid creating unnecessary environment
9ee074e
PR-URL: #58053
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
aduh95 pushed a commit that referenced this pull request May 17, 2025
@anonrig@aduh95
buffer: avoid creating unnecessary environment
8ba2823
PR-URL: #58053
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
aduh95 pushed a commit that referenced this pull request May 18, 2025
@anonrig@aduh95
buffer: avoid creating unnecessary environment
e95a83e
PR-URL: #58053
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
@github-actionsgithub-actionsBot mentioned this pull request May 19, 2025
Merged
aduh95 pushed a commit that referenced this pull request May 19, 2025
@anonrig@aduh95
buffer: avoid creating unnecessary environment
e7e8256
PR-URL: #58053
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
@ghostghost mentioned this pull request Jun 8, 2025
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes
@ronagronagronag approved these changes
@legendecaslegendecaslegendecas approved these changes
@juanarboljuanarboljuanarbol approved these changes
@marco-ippolitomarco-ippolitomarco-ippolito approved these changes

Assignees

No one assigned

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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@anonrig@nodejs-github-bot@jasnell@ronag@legendecas@juanarbol@marco-ippolito