Skip to content

src: do not shrink async resources on every pop - #66344

Open
nigrosimone wants to merge 1 commit into
nodejs:mainfrom
nigrosimone:async-resources-no-shrink
Open

nigrosimone wants to merge 1 commit into
nodejs:mainfrom
nigrosimone:async-resources-no-shrink

Conversation

@nigrosimone

@nigrosimone nigrosimone commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

AsyncHooks::pop_async_context() calls shrink_to_fit() on native_execution_async_resources_ after every pop. With the MSVC and libc++ standard libraries, shrink_to_fit() on an empty std::deque frees its storage, so every top-level callback scope allocates it again on push and frees it on pop. libstdc++ does nothing here, so on Linux nothing change.

Now the deque keeps its storage, like the async_ids_stack_ next to it, which never shrinks.

Until v24 the shrink ran only when the size was over 16 and under half of the capacity, so only after a deep nesting. Since #56457 it runs on every pop.

The same pattern in a small program, on Window with MSVC: 136-147 ns per callback with shrink_to_fit(), 6 ns without. macOS uses libc++, which frees the empty deque in the same way (not measured).

#include <chrono>
#include <cstdio>
#include <deque>
#include <variant>

std::deque<std::variant<int*, long*>> resources;
int value;

// Same as push_async_context() and pop_async_context() at depth 0.
__declspec(noinline) void Push() { resources.resize(1); resources[0] = &value; }
__declspec(noinline) void Pop() { resources.resize(0); resources.shrink_to_fit(); }

int main() {
  const int n = 5000000;
  auto start = std::chrono::steady_clock::now();
  for (int i = 0; i < n; i++) { Push(); Pop(); }
  auto end = std::chrono::steady_clock::now();
  printf("%.1f ns\n", std::chrono::duration<double, std::nano>(end - start).count() / n);
}

Refs: #66316
Refs: nodejs/performance#24

Disclosure: I used Opus 5.5 (Max) as coding assistant

AsyncHooks::pop_async_context() calls shrink_to_fit() on
native_execution_async_resources_ after every pop. With the MSVC and
libc++ standard libraries, shrink_to_fit() on an empty std::deque frees
its storage, so every top-level callback scope allocates it again on
push and frees it on pop. libstdc++ does nothing here.

A program with the same pattern, a std::deque of the same variant going
from 0 to 1 to 0 elements, takes 136-147 ns per callback on Windows
with MSVC, and 6 ns without shrink_to_fit(). The async_ids_stack_ next
to it never shrinks either.

Refs: nodejs/performance#24
Signed-off-by: Nigro Simone <nigro.simone@gmail.com>
@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 Sep 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Welcome to Node.js, and thank you for your first contribution!

Before review, please take a moment to read:

Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal.

@nigrosimone
nigrosimone marked this pull request as ready for review September 29, 2026 07:06
@codecov

codecov Bot commented Sep 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.37%. Comparing base (66f26d3) to head (a3a1b3c).
⚠️ Report is 57 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66344      +/-   ##
==========================================
- Coverage   90.38%   90.37%   -0.02%     
==========================================
  Files         790      792       +2     
  Lines      274497   275445     +948     
  Branches    52557    52781     +224     
==========================================
+ Hits       248100   248922     +822     
- Misses      16879    16936      +57     
- Partials     9518     9587      +69     
Files with missing lines Coverage Δ
src/env.cc 82.08% <ø> (-0.20%) ⬇️

... and 66 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.

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

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants