Revisit `_NameMap` in light of Swift 5 changing the default `String` encoding
thomasvl opened this issue · 1 comments
For a while now I've been wondering if _NameMap
should get a revisit since Swift 5 went UTF-8. We likely could drop all the allocations related to the InternPool
, and we might actually end up at the point the compiler can just realized everything is concurrency safe again.
But given the Swift 5.10 new concurrency warnings around it (#1560), it seems like that might be even more reason to consider revisiting/simplifying it.
Adding a little more to what I was wondering:
- Drop the
InternPool
- Maybe let
Name
continue with the enum storingStaticString
vs.String
; but this might not be needed (see below). - Stop
Name
from exposing theutf8Buffer
, and instead put an function on it to have it append to a[UInt8]
.- That can do the fast/slow paths for accessing the data.
- Update the encoders to use that api to write things out.
- Maybe even mark this one as
@inlinable
, since it would still beinternal
, we might not have too.
Since all the uses of _NameMap
should be from generated code, we'll get StaticStrings
for everything but the case we have to compute the json name from the proto name; and in that case, we likely could even assert that what we generated still passes isContiguousUTF8
and/or just call makeContiguousUTF8()
after making our String
. So Name
might not be needed at all, and _NameMap
might be able to just store String
directly.