-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[stdlib] Add fast string interpolation for metatypes #32113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[stdlib] Add fast string interpolation for metatypes #32113
Conversation
@swift-ci please test |
Build failed |
Build failed |
5bcdc82
to
215806f
Compare
@swift-ci please test |
@swift-ci please benchmark |
@swift-ci please test source compatibility |
Build failed |
Build failed |
Performance: -O
Code size: -OPerformance: -Osize
Code size: -OsizePerformance: -Onone
Code size: -swiftlibsHow 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 |
Performance: -O
Code size: -O
Performance: -Osize
Code size: -Osize
Performance: -Onone
Code size: -swiftlibsHow 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm okay with this change in principle, but I have some questions about the specifics.
215806f
to
aa52ed5
Compare
aa52ed5
to
7b5a4b9
Compare
@@ -471,7 +471,7 @@ var stringInterp = "\(#^STRING_INTERP_3^#)" | |||
_ = "" + "\(#^STRING_INTERP_4^#)" + "" | |||
// STRING_INTERP: Begin completions | |||
// STRING_INTERP-DAG: Decl[InstanceMethod]/CurrNominal/IsSystem: ['(']{#(value): T#}[')'][#Void#]; | |||
// STRING_INTERP-DAG: Decl[Struct]/CurrModule: FooStruct[#FooStruct#]; | |||
// STRING_INTERP-DAG: Decl[Struct]/CurrModule/TypeRelation[Convertible]: FooStruct[#FooStruct#]; name=FooStruct |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rintaro I tracked down the real difference and I think this is it. Does that make sense, given we've overloaded interpolation to take a metatype?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it makes sense 👍
@swift-ci please test |
@swift-ci please benchmark |
@swift-ci please benchmark |
Performance: -O
Code size: -O
Performance: -Osize
Code size: -Osize
Performance: -Onone
Code size: -swiftlibsHow 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
|
Build failed |
@swift-ci please smoke test Linux platform |
* Add fast string interpolation for metatypes (#32113) * Whitespace fixes Co-authored-by: Ben Cohen <[email protected]>
"\(T.self)"
is pretty common, and sometimes performance sensitive, but currently takes the slowest of paths. This gives them a custom string interpolation that cuts straight to what a long process of conformance checks and mirror reflection used to do.This makes
_typeName
ABI (it was already public but underscored) which I think is OK.