aio-libs / multidict / 36676991660

86%

push

github

Build a CIMultiDict key's istr from the key's own string (#1640)

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

## What do these changes do?

`CIMultiDict` returns every key as an `istr` (since #1097). For a key
added as a `str` subclass, both backends built that `istr` with
`istr(key)`, which calls the subclass's own `__str__`: a `Token("Key")`
whose `__str__` returns `"spoof"` read back as `istr("spoof")` while
lookups still matched `"key"`. The `istr` is now built from the key's
own string (`PyUnicode_FromObject()` in C, `str.__str__()` in pure
Python).

The C extension also caches the `istr` in place of the stored key on its
first read. It now does that only for an exact `str`, whose release runs
no code; a `str` subclass key is converted on every read instead of
being released inside the read, where its `__del__` could run. The
caching moved into a `NOINLINE` helper, which makes key iteration
cheaper on both classes.

The docs now say that `CIMultiDict` converts keys to `istr`.

Converting keys to `istr` on insertion instead was measured and
rejected: `CIMultiDict(items)` was 3.9x (GIL) and 2.8x (FT) slower with
200 plain `str` keys, and every `str`-keyed insert 2x to 3.8x slower.

Callgrind, Ir per operation against an identically built master; every
row not listed moved less than 0.5%, including every pure-Python
`CIMultiDict` row:

| operation | GIL MultiDict | GIL CIMultiDict | FT MultiDict | FT
CIMultiDict |
| --- | ---: | ---: | ---: | ---: |
| `for k in d` | 59 to 53 (-10.2%) | 63 to 54 (-14.3%) | 102 to 87
(-14.7%) | 103 to 88 (-14.6%) |
| `for k, v in d.items()` | 128 to 122 (-4.7%) | 132 to 123 (-6.8%) |
187 to 172 (-8.0%) | 188 to 173 (-8.0%) |
| `d.popitem()` | | | | 1,616 to 1,628 (+0.7%) |

## Are there changes in behavior for the user?

Yes, for `str` subclass keys that override `__str__`: a `CIMultiDict`
now returns them spelled as the key itself. `MultiDict` is unchanged.

## Is it a substantial burden for the maintainers to support... (continued)

788 of 1576 branches covered (50.0%)

Branch coverage included in aggregate %.

34 of 34 new or added lines in 2 files covered. (100.0%)

8945 of 9621 relevant lines covered (92.97%)

1.86 hits per line