Skip to content

Commit 44a5bef

Browse files
committed
Optimize lookup overflow hot paths
1 parent 3fb29af commit 44a5bef

11 files changed

Lines changed: 690 additions & 241 deletions

File tree

.agents/sow/current/SOW-0021-20260613-netipc-at-scale.md

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,6 +1156,37 @@ Follow-up mapping:
11561156
- add any remaining broader adversarial tests from the planned matrix beyond the now-covered representative `8192`/`32768` logical-call cases, now-covered mid-logical timeout/abort cases, now-covered malformed follow-up responses after partial progress, now-covered reordered/duplicate response-item corruption, now-covered invalid status/status-dependent/label-table response corruption, now-covered huge valid label isolation cases, now-covered endpoint absence before call, now-covered endpoint disappearance after partial progress, now-covered endpoint disappearance before the first subcall, now-covered zero-item typed lookup calls, now-covered stale request-capacity reconnect cases, now-covered duplicate/unsorted request keys under request splitting, now-covered request cap-minus-one/exact/plus-one boundaries, now-covered exact response-fit plus/minus-one boundaries, now-covered no-progress overflow cases, now-covered raw no-growth overflow cases, now-covered logical response-byte ceilings, now-covered mixed-status cross-language interop cases, and now-covered lookup status codec interop cases;
11571157
- keep lookup-scale interop green across POSIX baseline, POSIX SHM, Windows Named Pipe, and Windows SHM; all four profiles now cover both all-known scale and mixed-status C/Rust/Go directed tests.
11581158

1159+
## Performance Checkpoint - 2026-06-15
1160+
1161+
Context:
1162+
1163+
- A direct benchmark sample showed `cgroups-lookup-mixed-256` at roughly C `61.6k`, Rust `43.7k`, Go `25.9k` requests/s.
1164+
- The gap was not accepted as normal runtime noise. The lookup hot paths were reviewed language-by-language before committing.
1165+
1166+
Confirmed causes fixed in this checkpoint:
1167+
1168+
- C, Rust, and Go all had an O(n^2)-style response-overflow suffix-fit check in lookup builders. This was replaced with precomputed suffix bytes for dispatch-owned builders.
1169+
- Rust and Go cgroups dispatch paths called `request.Item(i)` in the payload-exceeded sizing prepass only to recover the already-validated request directory length. Both now read that length directly.
1170+
- Go response validation built item view objects while validating each response item. Go now has validate-only lookup item paths separate from public view construction.
1171+
- Go request views now cache the request packed-start offset instead of recomputing it on each item access.
1172+
1173+
Focused validation:
1174+
1175+
- `cd src/go && go test -count=1 ./pkg/netipc/protocol` passed.
1176+
- `cargo test --manifest-path src/crates/netipc/Cargo.toml protocol::lookup -- --test-threads=1` passed.
1177+
- `cmake --build build --target test_protocol -j24 && build/bin/test_protocol` passed with `514 passed, 0 failed`.
1178+
1179+
Sampled benchmark evidence after fixes, using release benchmark binaries:
1180+
1181+
- `cgroups-lookup-mixed-256`: C about `63k`, Rust about `48k`, Go about `34k-35k` requests/s.
1182+
- `apps-lookup-mixed-256`: C about `69k`, Rust about `62k`, Go about `40k-42k` requests/s.
1183+
1184+
Current interpretation:
1185+
1186+
- Rust is no longer broadly suspicious: `apps-lookup-mixed-256` is close to C, and the cgroups-specific avoidable overhead was reduced.
1187+
- Go remains materially slower than C/Rust in these codec+dispatch microbenchmarks. The obvious avoidable algorithmic and allocation issues found so far are fixed, but the remaining gap still needs full-suite benchmark confirmation and, if required, profiling before declaring it inherent Go/runtime overhead.
1188+
- This checkpoint does not close the performance gate. Full POSIX and Windows benchmark regeneration still needs to be run cleanly after commit/push.
1189+
11591190
## Downstream Vendoring Plan
11601191

11611192
Purpose:

src/crates/netipc/src/protocol/lookup/apps_lookup.rs

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ pub const APPS_LOOKUP_KEY_SIZE: usize = 8;
2323
#[derive(Debug)]
2424
pub struct AppsLookupRequestView<'a> {
2525
pub item_count: u32,
26+
packed_start: usize,
2627
payload: &'a [u8],
2728
}
2829

@@ -251,6 +252,7 @@ impl<'a> AppsLookupRequestView<'a> {
251252
}
252253
Ok(Self {
253254
item_count,
255+
packed_start: dir_end,
254256
payload: buf,
255257
})
256258
}
@@ -259,12 +261,6 @@ impl<'a> AppsLookupRequestView<'a> {
259261
if index >= self.item_count {
260262
return Err(NipcError::OutOfBounds);
261263
}
262-
let dir_size = (self.item_count as usize)
263-
.checked_mul(LOOKUP_DIR_ENTRY_SIZE)
264-
.ok_or(NipcError::BadItemCount)?;
265-
let packed_start = APPS_LOOKUP_REQ_HDR_SIZE
266-
.checked_add(dir_size)
267-
.ok_or(NipcError::BadItemCount)?;
268264
let base = APPS_LOOKUP_REQ_HDR_SIZE
269265
.checked_add(
270266
(index as usize)
@@ -274,7 +270,7 @@ impl<'a> AppsLookupRequestView<'a> {
274270
.ok_or(NipcError::BadItemCount)?;
275271
let off = u32_at(self.payload, base) as usize;
276272
Ok(u32_at(
277-
checked_subslice(self.payload, packed_start, off, APPS_LOOKUP_KEY_SIZE)?,
273+
checked_subslice(self.payload, self.packed_start, off, APPS_LOOKUP_KEY_SIZE)?,
278274
0,
279275
))
280276
}
@@ -450,7 +446,7 @@ pub struct AppsLookupBuilder<'a> {
450446
data_offset: usize,
451447
error: Option<NipcError>,
452448
payload_exceeded_suffix: bool,
453-
payload_exceeded_item_lens: Vec<usize>,
449+
payload_exceeded_suffix_bytes: Vec<usize>,
454450
}
455451

456452
impl<'a> AppsLookupBuilder<'a> {
@@ -471,7 +467,7 @@ impl<'a> AppsLookupBuilder<'a> {
471467
data_offset,
472468
error: None,
473469
payload_exceeded_suffix: false,
474-
payload_exceeded_item_lens: Vec::new(),
470+
payload_exceeded_suffix_bytes: Vec::new(),
475471
}
476472
}
477473

@@ -480,7 +476,10 @@ impl<'a> AppsLookupBuilder<'a> {
480476
}
481477

482478
pub fn set_payload_exceeded_item_lens(&mut self, item_lens: Vec<usize>) {
483-
self.payload_exceeded_item_lens = item_lens;
479+
match payload_exceeded_suffix_bytes_from_lens(item_lens) {
480+
Ok(suffix_bytes) => self.payload_exceeded_suffix_bytes = suffix_bytes,
481+
Err(err) => self.error = Some(err),
482+
}
484483
}
485484

486485
#[allow(clippy::too_many_arguments)]
@@ -579,7 +578,7 @@ impl<'a> AppsLookupBuilder<'a> {
579578
if !payload_exceeded_suffix_fits(
580579
self.buf.len(),
581580
item_end,
582-
&self.payload_exceeded_item_lens,
581+
&self.payload_exceeded_suffix_bytes,
583582
self.item_count + 1,
584583
self.max_items,
585584
) {
@@ -780,10 +779,14 @@ where
780779
}
781780
let mut builder = AppsLookupBuilder::new(resp, request.item_count, 0);
782781
if request.item_count > 0 {
783-
builder.set_payload_exceeded_item_lens(vec![
784-
APPS_LOOKUP_ITEM_HDR_SIZE + 3;
785-
request.item_count as usize
786-
]);
782+
let item_count = request.item_count as usize;
783+
let capacity = item_count.checked_add(1).ok_or(NipcError::Overflow)?;
784+
let mut item_lens = Vec::with_capacity(capacity);
785+
item_lens.resize(item_count, APPS_LOOKUP_ITEM_HDR_SIZE + 3);
786+
builder.set_payload_exceeded_item_lens(item_lens);
787+
if let Some(err) = builder.error {
788+
return Err(err);
789+
}
787790
}
788791
if !handler(&request, &mut builder) {
789792
return Err(builder.error().unwrap_or(NipcError::HandlerFailed));

src/crates/netipc/src/protocol/lookup/cgroups_lookup.rs

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ pub const CGROUPS_LOOKUP_ITEM_HDR_SIZE: usize = 28;
1616
#[derive(Debug)]
1717
pub struct CgroupsLookupRequestView<'a> {
1818
pub item_count: u32,
19+
packed_start: usize,
1920
payload: &'a [u8],
2021
}
2122

@@ -148,6 +149,7 @@ impl<'a> CgroupsLookupRequestView<'a> {
148149
}
149150
Ok(Self {
150151
item_count,
152+
packed_start: dir_end,
151153
payload: buf,
152154
})
153155
}
@@ -156,12 +158,6 @@ impl<'a> CgroupsLookupRequestView<'a> {
156158
if index >= self.item_count {
157159
return Err(NipcError::OutOfBounds);
158160
}
159-
let dir_size = (self.item_count as usize)
160-
.checked_mul(LOOKUP_DIR_ENTRY_SIZE)
161-
.ok_or(NipcError::BadItemCount)?;
162-
let packed_start = CGROUPS_LOOKUP_REQ_HDR_SIZE
163-
.checked_add(dir_size)
164-
.ok_or(NipcError::BadItemCount)?;
165161
let base = CGROUPS_LOOKUP_REQ_HDR_SIZE
166162
.checked_add(
167163
(index as usize)
@@ -172,10 +168,28 @@ impl<'a> CgroupsLookupRequestView<'a> {
172168
let off = u32_at(self.payload, base) as usize;
173169
let len = u32_at(self.payload, base + 4) as usize;
174170
Ok(StrView {
175-
bytes: checked_subslice(self.payload, packed_start, off, len)?,
171+
bytes: checked_subslice(self.payload, self.packed_start, off, len)?,
176172
len: (len - 1) as u32,
177173
})
178174
}
175+
176+
fn payload_exceeded_item_len(&self, index: u32) -> Result<usize, NipcError> {
177+
if index >= self.item_count {
178+
return Err(NipcError::OutOfBounds);
179+
}
180+
let base = CGROUPS_LOOKUP_REQ_HDR_SIZE
181+
.checked_add(
182+
(index as usize)
183+
.checked_mul(LOOKUP_DIR_ENTRY_SIZE)
184+
.ok_or(NipcError::BadItemCount)?,
185+
)
186+
.ok_or(NipcError::BadItemCount)?;
187+
let path_wire_len = u32_at(self.payload, base + 4) as usize;
188+
CGROUPS_LOOKUP_ITEM_HDR_SIZE
189+
.checked_add(path_wire_len)
190+
.and_then(|v| v.checked_add(1))
191+
.ok_or(NipcError::Overflow)
192+
}
179193
}
180194

181195
impl<'a> CgroupsLookupResponseView<'a> {
@@ -324,7 +338,7 @@ pub struct CgroupsLookupBuilder<'a> {
324338
data_offset: usize,
325339
error: Option<NipcError>,
326340
payload_exceeded_suffix: bool,
327-
payload_exceeded_item_lens: Vec<usize>,
341+
payload_exceeded_suffix_bytes: Vec<usize>,
328342
}
329343

330344
impl<'a> CgroupsLookupBuilder<'a> {
@@ -345,7 +359,7 @@ impl<'a> CgroupsLookupBuilder<'a> {
345359
data_offset,
346360
error: None,
347361
payload_exceeded_suffix: false,
348-
payload_exceeded_item_lens: Vec::new(),
362+
payload_exceeded_suffix_bytes: Vec::new(),
349363
}
350364
}
351365

@@ -354,7 +368,10 @@ impl<'a> CgroupsLookupBuilder<'a> {
354368
}
355369

356370
pub fn set_payload_exceeded_item_lens(&mut self, item_lens: Vec<usize>) {
357-
self.payload_exceeded_item_lens = item_lens;
371+
match payload_exceeded_suffix_bytes_from_lens(item_lens) {
372+
Ok(suffix_bytes) => self.payload_exceeded_suffix_bytes = suffix_bytes,
373+
Err(err) => self.error = Some(err),
374+
}
358375
}
359376

360377
pub fn add(
@@ -432,7 +449,7 @@ impl<'a> CgroupsLookupBuilder<'a> {
432449
if !payload_exceeded_suffix_fits(
433450
self.buf.len(),
434451
item_end,
435-
&self.payload_exceeded_item_lens,
452+
&self.payload_exceeded_suffix_bytes,
436453
self.item_count + 1,
437454
self.max_items,
438455
) {
@@ -619,16 +636,16 @@ where
619636
}
620637
let mut builder = CgroupsLookupBuilder::new(resp, request.item_count, 0);
621638
if request.item_count > 0 {
622-
let mut payload_exceeded_item_lens = Vec::with_capacity(request.item_count as usize);
639+
let item_count = request.item_count as usize;
640+
let capacity = item_count.checked_add(1).ok_or(NipcError::Overflow)?;
641+
let mut payload_exceeded_item_lens = Vec::with_capacity(capacity);
623642
for i in 0..request.item_count {
624-
let item = request.item(i)?;
625-
let item_len = CGROUPS_LOOKUP_ITEM_HDR_SIZE
626-
.checked_add(item.as_bytes().len())
627-
.and_then(|v| v.checked_add(2))
628-
.ok_or(NipcError::Overflow)?;
629-
payload_exceeded_item_lens.push(item_len);
643+
payload_exceeded_item_lens.push(request.payload_exceeded_item_len(i)?);
630644
}
631645
builder.set_payload_exceeded_item_lens(payload_exceeded_item_lens);
646+
if let Some(err) = builder.error {
647+
return Err(err);
648+
}
632649
}
633650
if !handler(&request, &mut builder) {
634651
return Err(builder.error().unwrap_or(NipcError::HandlerFailed));

src/crates/netipc/src/protocol/lookup/common.rs

Lines changed: 35 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -166,36 +166,49 @@ pub(super) fn lookup_dir_entry_offset(hdr_size: usize, index: u32) -> Result<usi
166166
.ok_or(NipcError::BadItemCount)
167167
}
168168

169+
pub(super) fn payload_exceeded_suffix_bytes_from_lens(
170+
mut item_lens: Vec<usize>,
171+
) -> Result<Vec<usize>, NipcError> {
172+
item_lens.push(0);
173+
for i in (0..item_lens.len() - 1).rev() {
174+
let next = item_lens[i + 1];
175+
let item_cost = if next > 0 {
176+
item_lens[i].checked_add(7).map(|v| v & !7)
177+
} else {
178+
Some(item_lens[i])
179+
}
180+
.ok_or(NipcError::Overflow)?;
181+
item_lens[i] = item_cost.checked_add(next).ok_or(NipcError::Overflow)?;
182+
}
183+
Ok(item_lens)
184+
}
185+
169186
pub(super) fn payload_exceeded_suffix_fits(
170187
buf_len: usize,
171-
mut data_offset: usize,
172-
item_lens: &[usize],
188+
data_offset: usize,
189+
suffix_bytes: &[usize],
173190
first_index: u32,
174191
max_items: u32,
175192
) -> bool {
176-
if item_lens.len() != max_items as usize {
193+
let max_items = max_items as usize;
194+
let Some(expected_len) = max_items.checked_add(1) else {
195+
return true;
196+
};
197+
if suffix_bytes.len() != expected_len {
177198
return true;
178199
}
179-
for item_len in item_lens
180-
.iter()
181-
.take(max_items as usize)
182-
.skip(first_index as usize)
183-
{
184-
let Some(item_start) = data_offset.checked_add(7).map(|v| v & !7) else {
185-
return false;
186-
};
187-
if item_start > buf_len {
188-
return false;
189-
}
190-
let Some(item_end) = item_start.checked_add(*item_len) else {
191-
return false;
192-
};
193-
if item_end > buf_len {
194-
return false;
195-
}
196-
data_offset = item_end;
200+
let first_index = first_index as usize;
201+
if first_index > max_items {
202+
return true;
203+
}
204+
if data_offset > usize::MAX - 7 {
205+
return false;
206+
}
207+
let item_start = align8(data_offset);
208+
if item_start > buf_len {
209+
return false;
197210
}
198-
true
211+
suffix_bytes[first_index] <= buf_len - item_start
199212
}
200213

201214
pub(super) fn validate_labels(

0 commit comments

Comments
 (0)