-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
[SE-0206][stdlib] hash(into:)/hashValue cleanup #16154
Conversation
cdf602b
to
28d5607
Compare
@swift-ci please benchmark |
@swift-ci test |
|
Build comment file:Optimized (O)Regression (24)
Improvement (17)
No Changes (381)
Unoptimized (Onone)Regression (12)
Improvement (23)
No Changes (387)
Hardware Overview
|
stdlib/public/core/Character.swift
Outdated
/// your program. Do not save hash values to use during a future execution. | ||
@inlinable // FIXME(sil-serialize-all) | ||
public var hashValue: Int { | ||
public func hash(into hasher: inout Hasher) { |
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.
All other hash(into:) are inlinable but this one, is that 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.
Oops, good catch!
That's a surprisingly significant impact. If #16157 lands, then it will mostly hide this issue, but it will affect older user code. (The performance regression will still show up in the new One option is to leave the |
@swift-ci please benchmark |
@swift-ci smoke test |
Build comment file:Optimized (O)Regression (13)
Improvement (12)
No Changes (399)
Unoptimized (Onone)Regression (17)
Improvement (22)
No Changes (385)
Hardware Overview
|
These are now synthesized by the compiler. (Inlinability will be different, but that seems fine.)
The @inlinable attribute was left off by accident, but this turned out to be a measurable performance boost for Character hashing. Also add @effects(releasenone), for good measure.
0044bfe
to
bae6f52
Compare
@swift-ci test and merge |
Add
hash(into:)
implementations for stdlib types that currently lack them, and removehashValue
implementations that can be generated by the compiler.