# HeapVector Active-Iterator-Checks — Speedometer Hotspot Identification

Companion to [`cl_7810435_brp_iterator_speedometer3_regression.md`](cl_7810435_brp_iterator_speedometer3_regression.md).

## Goal

The existing `ENABLE_HEAP_VECTOR_ACTIVE_ITERATOR_CHECKS` (AIC) machinery in `vector.h` shows a **0.6% Speedometer regression** on the perf bots and is blocked from shipping by the Chromium **0.3% Speedometer regression threshold**. The goal of this experiment is to **identify the hot HeapVector iterator-copy call sites** so we can opt them out of the counter ops and bring the regression below 0.3%.

This is the HeapVector analogue of the analysis already done for non-Heap `WTF::Vector` in the CL 7810435/1 post-mortem.

## Hypothesis

The same pattern that showed up for non-Heap `Vector` will show up for `HeapVector`: a small number of STL-algorithm and hand-written iteration sites copy iterators many times per element, dominating the AIC cost. Specifically the suspects, by analogy:

- `std::sort` / `std::ranges::find` / `std::adjacent_find` over `UncheckedIterator<Member<X>>`
- `base::internal::Zipper<HeapVector<…>>` in style resolution (this one is HeapVector-native; it already showed in the non-Heap profile)
- `Seeker<…>::Seek`, layout-tree walks, GC scan paths
- HTML construction-site task queue iteration
- DisplayItemList traversal

If these dominate, opting them out (raw-pointer access via `data()` / `base::span`) should bring AIC to baseline-flat.

## The visibility problem

The current AIC code in `vector.h` (under `ENABLE_HEAP_VECTOR_ACTIVE_ITERATOR_CHECKS`) does the counter ops inline:

```cpp
if (active_iterator_count_) {
  ++(*active_iterator_count_);   // ~1-2 cycles, fully inlined
}
```

That's so cheap it disappears into whatever function called `begin()`/`end()`. Perf attribution lands on the *containing* function (e.g. `ParseSomething: +0.4%`) with no easy way to identify the iterator-copy hotspot inside. With ~0.6% total AIC cost spread across many sites, no single symbol pops out.

## The marker technique

Replace the cheap inline inc/dec with **NOINLINE wrapper functions that do deliberate busy work**, hosted out of line in a wtf source file. Each iterator construction/destruction now calls into a named symbol that takes ~50-100ns instead of ~2ns. The artificial slowdown inflates the regression (we don't care — this is identification only) and makes the marker function dominate in `perf report`, with crisp call-graph attribution back to every iterator-copy site.

This is the same trick the BRP-in-iterator CL accidentally pulled off — `AcquireVectorBackingBrpRef` was effectively a slow marker that exposed the non-Heap hot sites cleanly. For HeapVector we synthesize the same condition deliberately.

## Implementation

Changes live on top of main HEAD (the existing AIC code).

### `partition_allocator.h`

Add two static markers to `PartitionAllocator`:

```cpp
 public:
  // NOINLINE markers used to amplify the visibility of HeapVector active-
  // iterator counter bumps in CPU profiles. The cheap inline `++(*p)` /
  // `--(*p)` disappears into the calling function; these wrappers force the
  // work into a named symbol with clear call-graph attribution, so profilers
  // like perf can point at the hot iterator-copy call sites. Adds deliberate
  // busy work so each call is measurable. For hotspot identification only —
  // strip before shipping AIC.
  static void HeapVectorIterAcquireMarker(uint32_t* p);
  static void HeapVectorIterReleaseMarker(uint32_t* p);
```

### `partition_allocator.cc`

```cpp
NOINLINE void PartitionAllocator::HeapVectorIterAcquireMarker(uint32_t* p) {
  if (p) ++(*p);
  for (volatile int i = 0; i < 30; ++i) {
    asm volatile("" ::: "memory");
  }
}
NOINLINE void PartitionAllocator::HeapVectorIterReleaseMarker(uint32_t* p) {
  if (p) --(*p);
  for (volatile int i = 0; i < 30; ++i) {
    asm volatile("" ::: "memory");
  }
}
```

The busy loop is opaque to the optimizer (volatile + asm barrier) and adds ~30× the natural counter-op cost, so a 0.6%-total AIC regression becomes ~15-20% in the marker build. We don't care — this is only used to *find* the call sites, not to measure cost.

### `vector.h`

In the `ENABLE_HEAP_VECTOR_ACTIVE_ITERATOR_CHECKS` arm, replace every:

```cpp
++(*active_iterator_count_);
// or
--(*active_iterator_count_);
```

with:

```cpp
PartitionAllocator::HeapVectorIterAcquireMarker(active_iterator_count_);
// or
PartitionAllocator::HeapVectorIterReleaseMarker(active_iterator_count_);
```

There are 3 bump sites (`UncheckedIterator(T*, wtf_size_t*)`, copy ctor, copy-assign post-update) and 3 decrement sites (dtor, copy-assign pre-update, move-assign pre-update). All inside the AIC buildflag arm, so non-AIC builds are unchanged.

### `args.gn`

Build dir: `out/AICmark`.

```gn
is_debug=false
shim_supports_alloc_token=true
dcheck_always_on=false
enable_heap_vector_active_iterator_checks=true
```

DCHECKs off so the profile isn't polluted by `AssertLayoutTreeUpdatedForPseudoElements` and other DCHECK-only callers (lesson learned from the BRP-in-iterator investigation).

### Telemetry patches

Same three patches as the prior CL investigation, reapplied after `gclient sync`:

- `device_finder.py`: `TELEMETRY_SKIP_ANDROID=1` env to skip catapult's WSL2-broken adb scan.
- `desktop_browser_backend.py`: main HEAD's version already grabs the executable name dynamically (better than my prior hand-patched grep), conflict resolved by keeping main's version.
- `browser_interval_profiling_controller.py`: `TELEMETRY_PERF_PATH` env to use `/home/keishi/bin/perf` instead of `/usr/bin/perf` (which is a WSL2-incompatible Debian wrapper).

## Workflow

1. **Build content_shell** at main HEAD with AIC + markers (`out/AICmark/`). [In progress.]
2. **Run the 32-suite Speedometer 3 sweep** at iter=10, with perf attached:
   ```bash
   ./tools/perf/run_benchmark run speedometer3 \
     --browser=exact --browser-executable=out/AICmark/content_shell \
     --suite='^<NAME>$' --iterations=10 \
     --interval-profiling-period=story_run \
     --interval-profiling-target=renderer:main \
     --extra-browser-args="--disable-features=PartitionAllocDanglingPtr"
   ```
3. **Aggregate `HeapVectorIterAcquireMarker` callers** across all 32 perf.data files:
   ```bash
   perf report -i <suite>/perf.data --stdio --no-children \
     --call-graph=graph,1,callee --percent-limit=0.05 \
     | awk '/HeapVectorIterAcquireMarker/,/^[[:space:]]*$/'
   ```
4. **Build the opt-out candidate list.** For each top caller:
   - Confirm it's a short-lived in-stack iteration (no JS reentry, no callback escape, no container mutation).
   - Note the file and call-site.
5. **Build a clean AIC config** (markers off, real `++(*p)` / `--(*p)` restored) and re-measure against a no-AIC baseline. Hope for mean Δ < 0.3%.

A single build is sufficient for step 1 — we don't need a baseline comparison for hotspot identification because the marker symbol *is* the attribution signal.

## What we already expect to find (from the CL 7810435 sweep)

The earlier non-Heap Vector sweep already surfaced HeapVector callers — they ran in the same content_shell because both share the same `UncheckedIterator` template. The patterns visible across multiple suites were:

| Pattern | Suites where it dominated | Likely HeapVector relevance |
|---|---|---|
| `base::internal::Zipper<HeapVector<MatchedRule>>` | TodoMVC-Svelte-Complex-DOM, Charts-observable-plot, TodoMVC-WebComponents, TodoMVC-Vue, React-Stockcharts-SVG, TodoMVC-Lit-Complex-DOM | **HeapVector-native** — this is style cascade walking |
| `std::sort<UncheckedIterator<MatchedRule>>` | TodoMVC-React-Redux, TodoMVC-WebComponents-Complex-DOM | HeapVector — cascade ordering |
| `ElementRuleCollector::CollectMatchingRulesForListInternal` | TodoMVC-Angular, TodoMVC-Preact, TodoMVC-Preact-Complex-DOM, TodoMVC-React-Redux-Complex-DOM | HeapVector — match collection |
| `ElementRuleCollector::SortMatchedRules` | Perf-Dashboard | HeapVector — explicit sort |
| `ScopedStyleResolver::CollectMatchingElementScopeRules` | TodoMVC-Backbone-Complex-DOM | HeapVector — scope walking |
| `MatchedPropertiesCache::Find` | TodoMVC-Angular-Complex-DOM | HeapVector — cache walk |
| `DisplayItemList::ItemsInRange` / `AppendSubsequenceByMoving` | TodoMVC-React-Complex-DOM, TodoMVC-Vue-Complex-DOM | HeapVector — paint records |
| `Seeker<ContainerQuerySet>::Seek` | NewsSite-Next | HeapVector — container queries |
| `HTMLConstructionSite::ExecuteQueuedTasks` | TodoMVC-Lit | Maybe HeapVector — parser task queue |
| `ScriptRunIterator::MergeSets` → `std::ranges::__find` | Editor-TipTap | Almost certainly HeapVector |

The marker sweep should confirm and rank these. We already have the design language for fixing them — same `data()` / `base::span` opt-out as for the non-Heap case.

## Connection to the broader design discussion

Per the design discussion documented in [the CL 7810435 writeup](cl_7810435_brp_iterator_speedometer3_regression.md), the strategic direction landing seems to be:

- Two iterator types in `Vector`: cheap `iterator` (raw `T*`) as default, counter-protected `safe_iterator` opt-in.
- A clang AST matcher rewrites for-loops to use `safe_iterator`; STL and hand-written read-only iteration use the cheap one.
- Crash-on-mutate-while-iterating as the mitigation primitive (matches existing HeapVector AIC).

This experiment's output — the ranked list of hot HeapVector iterator-copy sites — feeds directly into that plan. Those sites become the **explicit opt-out candidates** for the safe-default policy, or the **explicit opt-in skip list** for the cheap-default policy. Either way, we need the list.

## Results (sweep complete)

### Per-suite AIC marker totals

Acquire + Release of `HeapVectorIter*Marker` as % of renderer-main-thread CPU time. The marker has a ~30× artificial cost amplifier (busy loop), so the *real* AIC cost on each suite is roughly these numbers divided by 30. The 11.4% top number corresponds to ~0.38% real cost per suite — consistent with the 0.6% Speedometer-wide regression the perf bots reported.

| Suite | Acquire% | Release% | Combined |
|---|---:|---:|---:|
| TodoMVC-WebComponents-Complex-DOM | 5.67 | 5.76 | **11.43** |
| TodoMVC-Lit-Complex-DOM | 4.67 | 5.14 | 9.81 |
| TodoMVC-Preact-Complex-DOM | 4.42 | 5.34 | 9.76 |
| TodoMVC-Svelte-Complex-DOM | 4.22 | 5.33 | 9.55 |
| TodoMVC-Vue-Complex-DOM | 3.73 | 4.79 | 8.52 |
| TodoMVC-JavaScript-ES6-Webpack-Complex-DOM | 3.95 | 4.21 | 8.16 |
| TodoMVC-Backbone-Complex-DOM | 3.87 | 4.19 | 8.06 |
| NewsSite-Next | 3.71 | 4.24 | 7.95 |
| TodoMVC-JavaScript-ES5-Complex-DOM | 3.73 | 4.13 | 7.86 |
| TodoMVC-WebComponents | 3.33 | 4.26 | 7.59 |
| TodoMVC-React-Redux-Complex-DOM | 3.35 | 3.96 | 7.31 |
| NewsSite-Nuxt | 3.02 | 4.05 | 7.07 |
| TodoMVC-Lit | 2.91 | 3.98 | 6.89 |
| TodoMVC-React-Complex-DOM | 3.24 | 3.52 | 6.76 |
| TodoMVC-Angular-Complex-DOM | 3.04 | 3.24 | 6.28 |
| Charts-observable-plot | 2.34 | 2.58 | 4.92 |
| TodoMVC-Svelte | 2.58 | 2.64 | 5.22 |
| TodoMVC-Preact | 2.40 | 2.84 | 5.24 |
| TodoMVC-Vue | 2.26 | 2.66 | 4.92 |
| TodoMVC-jQuery-Complex-DOM | 1.96 | 2.39 | 4.35 |
| TodoMVC-JavaScript-ES6-Webpack | 1.56 | 2.38 | 3.94 |
| TodoMVC-Backbone | 1.80 | 2.56 | 4.36 |
| TodoMVC-JavaScript-ES5 | 1.75 | 2.09 | 3.84 |
| Editor-TipTap | 2.01 | 1.89 | 3.90 |
| React-Stockcharts-SVG | 1.53 | 1.85 | 3.38 |
| Perf-Dashboard | 1.52 | 1.69 | 3.21 |
| TodoMVC-React-Redux | 1.52 | 1.33 | 2.85 |
| TodoMVC-React | 1.37 | 1.61 | 2.98 |
| TodoMVC-Angular | 1.06 | 1.02 | 2.08 |
| Editor-CodeMirror | 0.89 | 0.84 | 1.73 |
| TodoMVC-jQuery | 0.92 | 0.87 | 1.79 |
| Charts-chartjs | 0.12 | 0.16 | 0.28 |

The pattern is **uniform**: the Complex-DOM variants take the biggest hits (more elements → more style recalc → more iterator copies in the style cascade), and `chartjs` shows almost no AIC traffic (its workload is canvas-based, no DOM churn). Suites with substantial regression but not Complex-DOM (NewsSite-Next, TodoMVC-WebComponents) likely have heavy class-name churn.

### Cross-suite top callers

Aggregated across all 32 suites — for each direct caller of `HeapVectorIterAcquireMarker`, sum of the %s it claimed and how many suites it appeared in. Sorted by total %.

| Rank | Caller | Suites | Σ% | Mean% | Type |
|---:|---|---:|---:|---:|---|
| 1 | **`blink::Seeker<ContainerQuerySet>::Seek(unsigned int)`** | 30 | 20.31 | 0.68 | Single hand-written walk |
| 2 | `blink::ContainerNode::RecalcDescendantStyles` (recursion fan-out) | 186 | 19.16 | 0.10 | Style-recalc tree recursion |
| 3 | `blink::StyleResolver::MatchUARules(Element&, ElementRuleCollector&)` | 57 | 8.49 | 0.15 | Style match collection |
| 4 | `blink::Element::OriginalStyleForLayoutObject(StyleRecalcContext&)` | 78 | 8.45 | 0.11 | Style resolution |
| 5 | **`blink::ElementRuleCollector::CollectMatchingRulesForListInternal`** | 29 | 7.90 | 0.27 | Single hot call site |
| 6 | **`std::__sort_impl<UncheckedIterator<MatchedRule>, ElementRuleCollector::CompareRules>`** | 25 | 3.54 | 0.14 | Cascade sort |
| 7 | **`blink::ScopedStyleResolver::CollectMatchingElementScopeRules`** | 23 | 2.89 | 0.13 | Scope walking |
| 8 | **`blink::Fullscreen::FullscreenElementFrom(Document&)`** | 22 | 2.75 | 0.12 | Fullscreen stack walk |
| 9 | **`blink::MatchedPropertiesCache::Find`** | 20 | 2.66 | 0.13 | Cache lookup |
| 10 | **`blink::Document::ActiveModalDialog() const`** | 21 | 2.07 | 0.10 | Dialog stack walk |
| 11 | `blink::StyleEngine::RecalcStyle` (recursion-style) | 18 | 1.35 | 0.08 | Style-recalc entry |
| 12 | **`std::ranges::__find::__find_unwrap` over `UncheckedIterator<...>`** | 15 | 1.20 | 0.08 | Range-find |
| 13 | **`blink::ElementRuleCollector::SortAndTransferMatchedRules`** | 14 | 1.19 | 0.08 | Cascade sort+xfer |
| 14 | `blink::StyleCascade::ResolveSubstitutions` | 15 | 1.14 | 0.08 | Cascade evaluation |
| 15 | `blink::StyleResolver::MatchAuthorRules` | 14 | 1.14 | 0.08 | Style match collection |
| 16 | **`blink::ElementRuleCollector::SortMatchedRules()`** | 12 | 1.13 | 0.09 | Cascade sort |
| 17 | **`blink::(anonymous namespace)::LastItemToCollapseWith`** | 13 | 1.10 | 0.08 | Custom helper |
| 18 | `blink::Element::StyleForPseudoElement` | 17 | 1.10 | 0.06 | Style resolution |
| 19 | `blink::ElementRuleCollector::CollectMatchingRulesInternal` | 14 | 1.02 | 0.07 | Match collection |
| 20 | **`blink::CSSSelectorParser::ResetVectorAfterScope::AddedElements()`** | 4 | 0.83 | 0.21 | Single helper |
| 21 | **`blink::CachedMatchedProperties::RefreshKey/CorrespondsTo`** | 10×2 | 1.50 | 0.07 | Cache compare |
| 22 | **`blink::LineBreaker::RewindTrailingOpenTags(LineInfo*)`** | 4 | 0.37 | 0.09 | Layout |

**Bold = concrete single-site opt-out candidates.** Non-bold entries are recursion fan-outs or generic style-recalc machinery — fixing them needs structural change, not a per-call-site patch.

### Opt-out priority list

Ranked by total cross-suite impact, with target file/symbol:

1. **`blink::Seeker<ContainerQuerySet>::Seek`** — `third_party/blink/renderer/core/css/container_query_evaluator.cc` (or similar). Hit in 30/32 suites, mean 0.68% per suite. **By far the biggest single win.**
2. **`blink::ElementRuleCollector::CollectMatchingRulesForListInternal`** — `third_party/blink/renderer/core/css/resolver/element_rule_collector.cc`. 29 suites, mean 0.27%.
3. **`std::sort<UncheckedIterator<MatchedRule>, ElementRuleCollector::CompareRules>`** — the cascade sort. 25 suites, mean 0.14%. Pattern matches the non-Heap `HTMLFastPathParser` std::sort case exactly — same fix.
4. **`blink::ScopedStyleResolver::CollectMatchingElementScopeRules`** — `scoped_style_resolver.cc`. 23 suites, mean 0.13%.
5. **`blink::Fullscreen::FullscreenElementFrom`** — `fullscreen.cc`. 22 suites, mean 0.12%. Walks `HeapVector<Member<Element>>` of the fullscreen stack — short-lived, read-only.
6. **`blink::MatchedPropertiesCache::Find`** — `matched_properties_cache.cc`. 20 suites, mean 0.13%.
7. **`blink::Document::ActiveModalDialog`** — `document.cc`. 21 suites, mean 0.10%. Walks the modal dialog stack.
8. **`std::ranges::__find_unwrap<UncheckedIterator<...>>`** — multiple call sites; ~15 suites, mean 0.08%. Need to identify the specific callers.
9. **`blink::ElementRuleCollector::SortAndTransferMatchedRules` / `SortMatchedRules`** — same file as #2. Same shape as #3, same fix.
10. **`blink::CachedMatchedProperties::RefreshKey` / `CorrespondsTo`** — `matched_properties_cache.cc`. 10+10 suites.
11. **`blink::LineBreaker::RewindTrailingOpenTags`** — `line_breaker.cc`. Lower impact but distinct site.
12. **`blink::CSSSelectorParser::ResetVectorAfterScope::AddedElements`** — `css_selector_parser.cc`. Only 4 suites but high mean (0.21%).

Fixing the top-3 alone (Seeker, CollectMatchingRulesForListInternal, std::sort<MatchedRule>) accounts for **~32% of the marker's total cross-suite impact**, which translates to roughly **0.2 percentage points off the 0.6% Speedometer regression**. Top-10 likely buys the remaining ~0.1-0.2 pp needed to hit the 0.3% threshold.

### What's NOT a fixable site

- `ContainerNode::RecalcDescendantStyles`, `StyleEngine::RecalcStyle`, `Element::OriginalStyleForLayoutObject`, `StyleResolver::ResolveStyle`, `Element::RecalcStyle` — these are style-recalc recursion fan-outs. They show up with high cumulative % because the recursion touches many tree positions, but each position only does a small amount of iteration. Fixing the leaf callers (the named ones in the opt-out list) reduces these automatically. Don't try to "fix" the recursion itself.

## Status

- ✅ Telemetry patches restored after `gclient sync`.
- ✅ Marker functions added to `partition_allocator.{h,cc}`.
- ✅ `vector.h` AIC arms patched to call markers (3 acquire sites + 3 release sites).
- ✅ `out/AICmark` configured with AIC + DCHECK-off.
- ✅ content_shell built (`out/AICmark/content_shell`, 353 MB).
- ✅ 32-suite × iter=10 perf sweep complete (`/tmp/sm3-aic-marker/`).
- ✅ Cross-suite top-caller table aggregated.
- ⏳ Opt-out audit + patches (per-site work, see priority list above).
- ⏳ Clean AIC remeasure after opt-outs — markers stripped, real `++(*p)` restored. Target: mean Δ < 0.3%.

## Data and artifact locations

- Source: `/mnt/black/chromium/src` at main HEAD with marker patch applied (uncommitted).
- Build: `out/AICmark/`
- Per-suite perf data target: `/tmp/sm3-aic-marker/<Suite>/perf.data`
- Telemetry patches: still uncommitted in `third_party/catapult/telemetry/…`
- Marker-build content_shell binary path (once built): `out/AICmark/content_shell`

## Followups once the sweep completes

1. **Per-suite top-caller list** in this doc, replacing the "what we expect to find" table with actual data.
2. **Opt-out audit** — for each top caller, manually verify: short-lived, no JS reentry, no container mutation.
3. **Patch the opt-outs** — likely a series of small CLs per call site.
4. **Clean AIC remeasure** — same sweep with markers stripped, AIC code restored to plain `++(*p)`. Mean Δ target: <0.3%.
5. **Update this doc with the after numbers** and feed into the design discussion / CL stack.