IRGen: Access concrete type metadata by mangled name. #26455
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci Please test |
|
@swift-ci Please test compiler performance |
|
@swift-ci Please benchmark |
Performance: -O
Code size: -O
Performance: -Osize
Code size: -Osize
Performance: -Onone
Code size: -swiftlibs
How to read the dataThe tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.If you see any unexpected regressions, you should consider fixing the Noise: Sometimes the performance results (not code size!) contain false Hardware Overview |
|
@swift-ci Please benchmark |
|
Build failed |
Performance: -O
Code size: -O
Performance: -Osize
Code size: -Osize
Performance: -Onone
Code size: -swiftlibs
How to read the dataThe tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.If you see any unexpected regressions, you should consider fixing the Noise: Sometimes the performance results (not code size!) contain false Hardware Overview |
|
The performance numbers look reasonable to my untrained eye; the ones that are out of the 1x band all look too variable to get much signal from. @palimondo would you agree? |
|
Build failed |
jckarter
force-pushed
the
type-metadata-by-mangled-name
branch
from
84c0ba9 to
a70df24
Compare
|
@swift-ci Please test |
|
@swift-ci Please test source compatibility |
|
@swift-ci Please test compiler performance |
|
Build failed |
|
Build failed |
jckarter
force-pushed
the
type-metadata-by-mangled-name
branch
from
a70df24 to
2dd6697
Compare
|
@swift-ci Please test |
|
@swift-ci Please test source compatibility |
|
Build failed |
|
Build failed |
|
@swift-ci Please test compiler performance |
|
@swift-ci Please test Linux |
jckarter
force-pushed
the
type-metadata-by-mangled-name
branch
from
2dd6697 to
1dfaa02
Compare
|
@swift-ci Please benchmark |
|
@swift-ci Please test source compatibility |
Performance: -O
Code size: -O
Performance: -Osize
Code size: -Osize
Performance: -Onone
Code size: -swiftlibs
How to read the dataThe tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.If you see any unexpected regressions, you should consider fixing the Noise: Sometimes the performance results (not code size!) contain false Hardware Overview |
|
That’s a lot of numbers from the benchmarks! At first glance: The most recent run (after the force-push) looks much less intrusive than the first two. Should I look more closely? |
|
The regression in |
|
Also the regressions and improvements in |
|
@palimondo |
|
Source compat failure in RxSwift looks unrelated: |
I'm looking into it, but my general stance is: there are no noisy benchmarks. |
|
Just to clarify, the first two benchmark runs here were done on identical code, correct? If so, I really have a beef with the |
|
@palimondo Yeah, I ran it twice on the same code to begin with. (For the third one, I was more interested in the code size benchmarks.) |
|
Did you see anything similarly strange when running benchmarks locally? (Did you use |
|
@palimondo I hadn't tried running locally. I'll give that a shot. |
|
@swift-ci Please benchmark |
Performance: -O
Code size: -O
Performance: -Osize
Code size: -Osize
Performance: -Onone
Code size: -swiftlibs
How to read the dataThe tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.If you see any unexpected regressions, you should consider fixing the Noise: Sometimes the performance results (not code size!) contain false Hardware Overview |
|
@swift-ci Please benchmark |
|
JFC! I'll have more robust |
Performance: -O
Code size: -O
Performance: -Osize
Code size: -Osize
Performance: -Onone
Code size: -swiftlibs
How to read the dataThe tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.If you see any unexpected regressions, you should consider fixing the Noise: Sometimes the performance results (not code size!) contain false Hardware Overview |
When we generate code that asks for complete metadata for a fully concrete specific type that doesn't have trivial metadata access, like `(Int, String)` or `[String: [Any]]`, generate a cache variable that points to a mangled name, and use a common accessor function that turns that cache variable into a pointer to the instantiated metadata. This saves a bunch of code size, and should have minimal runtime impact, since the demangling of any string only has to happen once. This mostly just works, though it exposed a couple of issues: - Mangling a type ref including objc protocols didn't cause the objc protocol record to get instantiated. Fixed as part of this patch. - The runtime type demangler doesn't correctly handle retroactive conformances. If there are multiple retroactive conformances in a process at runtime, then even though the mangled string refers to a specific conformance, the runtime still just picks one without listening to the mangler. This is left to fix later, rdar://problem/53828345. There is some more follow-up work that we can do to further improve the gains: - We could improve the runtime-provided entry points, adding versions that don't require size to be cached, and which can handle arbitrary metadata requests. This would allow for mangled names to also be used for incomplete metadata accesses and improve code size of some generic type accessors. However, we'd only be able to take advantage of the new entry points in OSes that ship a new runtime. - We could choose to always symbolic reference all type references, which would generally reduce the size of mangled strings, as well as make runtime demangling more efficient, since it wouldn't need to hit the runtime caches. This would however require that we be able to handle symbolic references across files in the MetadataReader in order to avoid regressing remote mirror functionality.
On i386 Darwin, the linker appears to garble indirect symbolic references.
jckarter
force-pushed
the
type-metadata-by-mangled-name
branch
from
93f673d to
5d05809
Compare
|
@swift-ci Please benchmark |
|
@swift-ci Please test |
|
Build failed |
|
Build failed |
Performance: -O
Code size: -O
Performance: -Osize
Code size: -Osize
Performance: -Onone
Code size: -swiftlibs
How to read the dataThe tables contain differences in performance which are larger than 8% and differences in code size which are larger than 1%.If you see any unexpected regressions, you should consider fixing the Noise: Sometimes the performance results (not code size!) contain false Hardware Overview |
|
@swift-ci Please test Linux |
|
Build failed |
|
@swift-ci Please test Linux |
|
So I investigated the size and performance regressions, and they look like about what I'd expect—increased one-time initialization time from demangling, and size increases from the added mangled names themselves. In both cases, the effect only really seems noticeable in smaller programs, and I have followup work planned to mitigate both issues. Having mangled names always symbolic-reference nominal types ought to significantly decrease both demangling time and mangled string size. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When we generate code that asks for complete metadata for a fully concrete specific type that
doesn't have trivial metadata access, like
(Int, String)or[String: [Any]],generate a cache variable that points to a mangled name, and use a common accessor function
that turns that cache variable into a pointer to the instantiated metadata. This saves a bunch
of code size, and should have minimal runtime impact, since the demangling of any string only
has to happen once.
This mostly just works, though it exposed a couple of issues:
instantiated. Fixed as part of this patch.
multiple retroactive conformances in a process at runtime, then even though the mangled string
refers to a specific conformance, the runtime still just picks one without listening to the
mangler. This is left to fix later, rdar://problem/53828345.
There is some more follow-up work that we can do to further improve the gains:
to be cached, and which can handle arbitrary metadata requests. This would allow for mangled
names to also be used for incomplete metadata accesses and improve code size of some generic
type accessors. However, we'd only be able to take advantage of the new entry points in
OSes that ship a new runtime.
the size of mangled strings, as well as make runtime demangling more efficient, since it wouldn't
need to hit the runtime caches. This would however require that we be able to handle symbolic
references across files in the MetadataReader in order to avoid regressing remote mirror
functionality.