aio-libs / multidict / 37673564418

86%

push

github

Rewrite md_repr() on top of md_walk_all() (#1742)

<!-- Thank you for your contribution! -->

## What do these changes do?

Rewrites the C extension's `md_repr()` on top of `md_walk_all()`; the
loop body becomes a visitor, `_md_repr_visit()`. Keys are fetched only
when shown, so the values view never touches them.

Two follow-ups were needed. Once smaller, `_md_repr_locked()` got
inlined into all five `tp_repr` slots and pushed
`_compact_entry_matches()` out of line on FT (rule #1653), so it is now
`COLD`. The walk hands a CIMultiDict's keys over as cached `istr`, and
`PyUnicodeWriter_WriteStr()` copies a str subclass through `str()`, so
such a key is written with `PyUnicodeWriter_WriteSubstring()` instead.

This is a refactor with a measurable cost: the generic walk does more
per entry than the old loop (identity, hash and an indirect call).
Callgrind, 100 entries, instructions per `repr()` on 3.14.7:

| repr of | GIL before | GIL after | FT before | FT after |
|---|---|---|---|---|
| MultiDict | 139,362 | 142,615 (+2.3%) | 139,088 | 146,083 (+5.0%) |
| CIMultiDict | 139,507 | 143,290 (+2.7%) | 139,193 | 146,739 (+5.4%) |
| keys view | 41,954 | 46,930 (+11.9%) | 43,818 | 50,186 (+14.5%) |
| values view | 104,637 | 107,464 (+2.7%) | 104,155 | 108,005 (+3.7%) |

`md_add_with_hash_steal_refs()` also becomes a plain `static` function.
The repr rewrite shrinks the unit GCC sizes its inlining allowance from,
and with GCC 13 on FT that left `_compact_entry_matches()` out of line
(rule #1653; master already fails it at `--param
inline-unit-growth=38`). Outlining the add helper gives the budget back.
Callgrind driver, rows that moved by 2% or more against master:

| Operation | GIL | FT |
|---|---|---|
| `getitem_hit` | -8.8% / -9.3% | ±0 |
| `getitem_istr` (CI) | -10.5% | ±0 |
| `iter_keys` | ±0 | -16.2% / -16.8% |
| `iter_items` | ±0 | -8.8% / -8.9% |
| `update` | -2.0% / -2.7% | -0.8% / -1.3% |
| `setitem_replace` | -2.8% / -3.4% | ±0 |
| `add` | +2.1% / +... (continued)

793 of 1586 branches covered (50.0%)

Branch coverage included in aggregate %.

9353 of 10217 relevant lines covered (91.54%)

1.83 hits per line