aio-libs / multidict / 37194502182

86%

push

github

Return the keyword-bound arguments of parse2() by value (#1664)

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

## What do these changes do?

`parse2()` binds the key and the optional second argument of `get()`,
`getone()`, `getall()`, `add()`, `setdefault()`, `pop()`, `popone()` and
`popall()`. Its cold slow path, taken only for keyword calls, wrote both
results through pointers to the caller's locals. That made the locals
address-taken in every caller, so under `-fstack-protector-strong`
(Ubuntu's default, and what the CodSpeed runners build with) each of
those methods paid for a stack canary, two zeroing stores and reloads
from the stack on every call.

The slow path now returns both arguments by value in a two-pointer
struct, which comes back in registers on x86-64; `arg1` is `NULL` on
error, since a successful bind always sets it. The positional fast path
is unchanged. On the GIL build the hot functions with a canary went from
38 to 26. On the FT build the count does not move, since
`Py_BEGIN_CRITICAL_SECTION` hands a local struct to CPython anyway, so
the gain there is smaller.

## Are there changes in behavior for the user?

No.

## Is it a substantial burden for the maintainers to support this?

No. The change is confined to `parser.h`.

## Related issue number

N/A

## Checklist

- [x] I think the code is well written
- [x] Unit tests for the changes exist
- [x] Documentation reflects the changes (N/A, internal)
- [ ] If you provide code modification, please add yourself to
`CONTRIBUTORS.txt` (N/A)
- [x] Add a new news fragment into the `CHANGES/` folder
- [x] `make doc-spelling` passes and any new technical words are added
to `docs/spelling_wordlist.txt`

<details>
<summary>Agent run details (optional, for reviewers)</summary>

Callgrind (`benchmarks/callgrind_driver.py --include-multidict-only`,
3.14.7 / 3.14.7t, GCC 15.2, base a8821735), Ir per op:

| Operation | GIL MultiDict | GIL CIMultiDict | FT MultiDict | FT
CIMultiDict |
|---|---|---|--... (continued)

792 of 1584 branches covered (50.0%)

Branch coverage included in aggregate %.

34 of 37 new or added lines in 1 file covered. (91.89%)

9300 of 10147 relevant lines covered (91.65%)

1.83 hits per line