-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Re-factor Codable.decode functions to reduce the standard library code size #10126
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
Conversation
@swift-ci please smoke test |
@itaiferber Itai, do you mind having a look? I just re-factored the slow path. No semantic changes are intended. |
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.
Thanks for putting this together! 😄 I think we can make this even simpler, though.
However: given that this error production is never inlined, what's the tradeoff here we'll be making in terms of size for speed? Is there any sort of noticeable slowdown because we're not inlining?
I also have to admit, I'm not familiar with the @_semantics
being applied here. What do they optimize for?
stdlib/public/core/Codable.swift
Outdated
@_semantics("optimize.sil.specialize.generic.never") | ||
@_semantics("optimize.sil.specialize.generic.partial.never") | ||
@inline(never) | ||
internal func decodeError<T: Decodable>(_ type: T.Type, forKey key: Key) throws -> T { |
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.
What's the point of this throwing the error instead of just returning a new error?
Can't this just be internal func _errorInDecoding(_ type: T.Type, forKey key: Key) -> DecodingError { ... }
?
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.
Good idea! I just tried to be as similar as possible to the original code.
stdlib/public/core/Codable.swift
Outdated
@_semantics("optimize.sil.specialize.generic.never") | ||
@_semantics("optimize.sil.specialize.generic.partial.never") | ||
@inline(never) | ||
internal func decodeError<T: Decodable>(_ type: T.Type) throws -> T { |
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.
Same comment here. Why pretend to return a T
when we can just return the error?
@itaiferber We are not inlining the slow path now. So, we pay a price of a generic function call. But since it is a slow path, it doesn't matter. The @_semantics are used to prevent the generic specializer from (partially) specializing the generic code for concrete types, because this would result in the code bloat in the object file. |
…e size Move the error reporting slow-path into a separate function, which is not inlined or specialized. This reduced the stdlib code size by almost 1%.
8605699
to
890c2d5
Compare
@itaiferber I address your review comments. Looks better now? |
@swift-ci please smoke test |
1 similar comment
@swift-ci please smoke test |
@itaiferber Let me know if it is OK to merge or if you have any further improvements suggestions. |
LGTM but I'd like to be sure with a full test, if you don't mind. |
@swift-ci Please test and merge |
Move the error reporting slow-path into a separate function, which is not inlined or specialized.
This reduced the stdlib code size by almost 1%.