Skip to content

src,dns: reserve vector capacity to avoid unnecessary reallocations#64763

Open
agape1225 wants to merge 2 commits into
nodejs:mainfrom
agape1225:reserve-vector-capacity
Open

src,dns: reserve vector capacity to avoid unnecessary reallocations#64763
agape1225 wants to merge 2 commits into
nodejs:mainfrom
agape1225:reserve-vector-capacity

Conversation

@agape1225

Copy link
Copy Markdown

Message::Serialize() and ParseTxtReply() both grow a LocalVector
via repeated push_back() without reserving capacity first, even
though the upper bound on the number of elements is known before the
loop starts. This adds reserve() in both places, following the
usual idiom of reserving upfront when the size (or a bound on it) is
already known, to avoid the repeated allocate/copy/free cycle that
happens each time capacity is exceeded.

Note: in Message::Serialize, transfer_list_v.length() is an upper
bound rather than an exact count (only ArrayBuffer entries get
pushed into array_buffers), so a transfer list with few or no
ArrayBuffers reserves capacity that ends up unused. No functional
change either way.

The array_buffers vector's upper bound is known before the loop starts
(transfer_list_v.length()), so reserve the capacity upfront to avoid
unnecessary reallocations.

Signed-off-by: agape1225 <49804691+agape1225@users.noreply.github.com>
The chunks vector's upper bound is known before the loop starts
(str_count from ares_dns_rr_get_abin_cnt), so reserve the capacity
upfront to avoid unnecessary reallocations.

Signed-off-by: agape1225 <49804691+agape1225@users.noreply.github.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. needs-ci PRs that need a full CI run. labels Jul 26, 2026
@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.13%. Comparing base (4a5eb1c) to head (403b55e).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64763      +/-   ##
==========================================
- Coverage   90.15%   90.13%   -0.02%     
==========================================
  Files         744      744              
  Lines      242517   242519       +2     
  Branches    45688    45695       +7     
==========================================
- Hits       218642   218600      -42     
- Misses      15358    15410      +52     
+ Partials     8517     8509       -8     
Files with missing lines Coverage Δ
src/cares_wrap.cc 62.93% <100.00%> (+0.02%) ⬆️
src/node_messaging.cc 82.88% <100.00%> (+0.01%) ⬆️

... and 26 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++. cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants