Skip to content

with_capacity - #39

Merged
jgarzik merged 11 commits into
mainfrom
updates
Nov 8, 2026
Merged

jgarzik merged 11 commits into
mainfrom
updates

Conversation

@jgarzik

@jgarzik jgarzik commented Nov 8, 2026

Copy link
Copy Markdown
Owner

No description provided.

jgarzik and others added 7 commits November 8, 2026 07:36
put_dirent grew the buffer from inside the readdir actor, under the
file system's directory spinlocks, so every growth was an atomic
allocation (up to 13 reallocations for a 32 KiB call). The limit is
already clamped to GETDENTS_CHUNK, so one allocation of that size,
taken before any lock, holds every entry the call can return.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dump_step grew each part from empty while it filled up to DUMP_SIZE,
about six reallocations under NlSock.dump for every part of an
`ip addr` / `ip route` dump. A part is bounded by DUMP_SIZE plus the
NLMSG_DONE that may follow it, so allocate that once.

Msg::str copied the string into a temporary Vec only to append the
NUL; write the header, bytes and NUL straight into the message.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PollContext started empty and grew its entry and file vectors one fd at
a time on the registering scan: log2(n) reallocations of each for a
large poll. Size both up front, fallibly (ENOMEM, as Linux's
poll_get_entry), for what will register: every pollfd for poll, the
fds set in any of the three sets for select (not nfds, which is usually
FD_SETSIZE with a few bits set), and nothing when the timeout is zero.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
add_entry scanned every entry already registered to skip a queue seen
before, which made the registering scan O(n^2) in nfds. Linux adds an
entry for every poll_wait call; a queue registered twice only wakes the
poller twice, and it re-scans either way. Do the same.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every read regenerates these files, and each started from an empty
buffer grown line by line. Size each once from what it lists: the
visible mounts times a typical line, and the slab caches times their
fixed-width line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
walk split the path into a Vec of components, which split + filter
cannot size exactly, so every open and stat allocated (and a deep path
reallocated). Step through the path with next_comp instead; a symlink
continues with the unwalked tail of the path, slashes trimmed, rather
than components joined back together.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Its five conversion opportunities are done (getdents, netlink dump
parts, poll contexts, /proc generators, the path walk) and its pointer
to a BUGS.md section was stale. What it taught that STYLE.md lacked,
allocating once and before a spinlock and why order 4 fails there, is
now a STYLE.md rule.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jgarzik
jgarzik requested a balanced review from Copilot November 8, 2026 07:45
@jgarzik jgarzik self-assigned this Nov 8, 2026
…e the lock

Review of the dump pre-size:
- A part sized DUMP_SIZE + NLMSG_DONE was 20 bytes past the largest
  slab class, an order-2 page block per part. The fill already leaves
  room for NLMSG_DONE, so DUMP_SIZE holds a whole part: one slab object.
- rcvbuf was charged the payload, so a short part in a large buffer
  pinned many times its charge. Charge the buffer as allocated, as
  Linux's skb truesize does.
- The buffer was still allocated under NlSock.dump, a spin lock.
  Allocate it before taking the lock, once a dump is seen to run.

Msg::str and Msg::attr now share one attribute-header writer.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jgarzik and others added 2 commits November 8, 2026 07:46
Review of the poll context pre-size:
- Failing to size the context failed the whole poll with ENOMEM, even
  with fds ready, and registrations past the size still grew
  infallibly. As in Linux (poll_wqueues.error), the pre-size is now best
  effort, every registration is fallible, the first failure stops
  further ones, and the poll returns ENOMEM only when it finds nothing
  ready rather than sleeping on queues it may have missed.
- poll sized for every pollfd, negative (ignored) ones included; it now
  counts fds >= 0. select counted bits past nfds in the last word.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review of the in-place walk: the lookahead for the last component was
computed and then recomputed at the loop head, so carry it. The tail
after a component is empty or starts at a slash, so it extends the
link target as is; trimming its slashes changed nothing (a trailing
slash is already in the walk flags).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

Copilot was unable to complete this review at the selected tier due to model policy or plan restrictions.

🟡 Changes recommended

The change still has moderate unresolved issues in core logic—an O(n²) path-walk loop in fs/namei.rs and a potentially panicking under-allocation in io/poll.rs—that need to be addressed before it can be safely approved.

2 open issues
What changed in this PR

This pull request focuses on reducing allocation churn and allocator work in hot paths by pre-sizing buffers (with_capacity) where sizes are known/bounded, and by removing unnecessary intermediate allocations across netlink, procfs, directory iteration, pathname walking, and poll/select setup.

Changes:

  • Pre-size transient output buffers for netlink dumps and multiple /proc/* generators to reduce reallocation overhead.
  • Remove an intermediate allocation in netlink string attribute encoding by writing attributes directly into the message buffer.
  • Rework path component iteration to avoid allocating/collecting components; add small unit tests.
  • Introduce/use a fallible PollContext::with_capacity to size poll/select context storage more predictably.
  • Update style guidance on allocation sizing (especially avoiding allocator work under locks); remove the standalone sizing note doc.

Outstanding review comments (must be addressed):

  • fs/namei.rs: Current component loop calls next_comp(rest) twice per component, turning path walking into O(n²) on many-component paths; keep a lookahead to scan once. (severity: moderate)
  • fs/proc/mounts.rs: show_mountinfo() clones mountpoint Vec<u8> per mount via vis.iter().cloned(); iterate by reference and only clone what’s needed (e.g., Arc<Mount>) to avoid undermining pre-sizing. (severity: moderate)
  • io/poll.rs: PollContext::with_capacity assumes 1 registration per fd, but some fds register multiple wait queues; entries.push(...) can exceed capacity and trigger an infallible grow/panic despite the ENOMEM intent—budget for multiple registrations per fd. (severity: moderate)
File Summary
net/​netlink/​mod.rs Pre-allocates dump buffer capacity for netlink multipart replies/dumps.
net/​netlink/​attr.rs Avoids temporary allocation when emitting NUL-terminated string attributes by encoding directly into the message buffer.
mm/​slab.rs Pre-sizes slabinfo() output string based on estimated output size/line count.
io/​poll.rs Adds fallible PollContext::with_capacity and adjusts registration behavior; capacity assumptions flagged in review comments.
fs/​proc/​mounts.rs Pre-sizes /proc/<pid>/{mounts,mountinfo} output buffers based on visible mount count; cloning behavior flagged in review comments.
fs/​poll.rs Uses PollContext::with_capacity sized from expected registrations for poll/select.
fs/​namei.rs Switches to allocation-free, in-place path component iteration; adds helper unit tests; loop complexity flagged in review comments.
fs/​dir.rs Pre-sizes getdents buffer to avoid allocations while directory locks are held.
doc/​STYLE.md Documents allocation sizing guidance, especially around avoiding allocator work under locks/atomic contexts.
doc/​res/​hk-with-capacity.md Removes the standalone sizing write-up (content consolidated into style guidance).

This pull request requires changes.

Review effort: Balanced

Comment thread fs/namei.rs Resolved
Comment thread io/poll.rs Resolved
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jgarzik
jgarzik merged commit 9e1f6b2 into main Nov 8, 2026
6 checks passed
@jgarzik
jgarzik deleted the updates branch November 8, 2026 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants