Skip to content

[stdlib] Eliminate _HasherCore protocol #20198

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

Merged
merged 3 commits into from
Nov 7, 2018

Conversation

lorentey
Copy link
Member

@lorentey lorentey commented Oct 31, 2018

(This is a spinoff of #20185; it was created because eliminating _HasherCore used to cause a code size regression.)

Hasher only supports a single algorithm, so _HasherCore is an unnecessary protocol. It wasn't ever intended to be ABI-visible; the expectation was that Hasher would become a resilient struct, and internal details wouldn't become hardcoded in the ABI.

Unfortunately Hasher resiliency did not happen, so we're setting its internal layout & types in stone. This PR sanitizes these internals, updating them to the brave new fragile Hasher world.

Types that are implementation details of Hasher are now tucked away inside it, unnecessary generics are eliminated, names are made more sensible:

Old name New name
protocol _HasherCore (removed)
struct _HasherTailBuffer struct Hasher._TailBuffer
struct _BufferingHasher<Core> struct Hasher._Core
struct Hasher._Core merged into struct Hasher._State
  • Hasher._State is low-level SipHash implementation; it includes the fragile hashing state and internal operations that update it.
  • Hasher._TailBuffer is a small buffer collecting bytes into word-size chunks that can be fed to _State.
  • Hasher._Core consists of a tail buffer and a state; it implements low-level Hasher operations. (It's a little like _StringGuts.) Public Hasher operations call into it.

@lorentey
Copy link
Member Author

@swift-ci benchmark

@swift-ci

This comment has been minimized.

@lorentey lorentey changed the title [stdlib] Eliminate _HasherCore protocol 🚱[DNM][stdlib] Eliminate _HasherCore protocol Nov 1, 2018
@lorentey lorentey changed the title 🚱[DNM][stdlib] Eliminate _HasherCore protocol [stdlib] Eliminate _HasherCore protocol Nov 6, 2018
@lorentey lorentey force-pushed the hasher-padding-redux2 branch from 43e9fd9 to 7a717d6 Compare November 6, 2018 14:12
@lorentey
Copy link
Member Author

lorentey commented Nov 6, 2018

With the new String representation, this is likely a viable change now.

@lorentey
Copy link
Member Author

lorentey commented Nov 6, 2018

@swift-ci please test

@lorentey
Copy link
Member Author

lorentey commented Nov 6, 2018

@swift-ci please benchmark

@swift-ci

This comment has been minimized.

@swift-ci
Copy link
Contributor

swift-ci commented Nov 6, 2018

Build comment file:

No performance and code size changes

How to read the data The 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 regressions before you merge the PR.

Noise: Sometimes the performance results (not code size!) contain false alarms. Unexpected regressions which are marked with '(?)' are probably noise. If you see regressions which you cannot explain you can try to run the benchmarks again. If regressions still show up, please consult with the performance team (@eeckstein).

Hardware Overview
  Model Name: Mac Pro
  Model Identifier: MacPro6,1
  Processor Name: 12-Core Intel Xeon E5
  Processor Speed: 2.7 GHz
  Number of Processors: 1
  Total Number of Cores: 12
  L2 Cache (per Core): 256 KB
  L3 Cache: 30 MB
  Memory: 64 GB

@lorentey
Copy link
Member Author

lorentey commented Nov 6, 2018

@swift-ci test

@swift-ci

This comment has been minimized.

@swift-ci

This comment has been minimized.

@lorentey lorentey force-pushed the hasher-padding-redux2 branch from 264be40 to 382807e Compare November 7, 2018 15:33
@lorentey
Copy link
Member Author

lorentey commented Nov 7, 2018

@swift-ci please smoke test and merge

@lorentey lorentey force-pushed the hasher-padding-redux2 branch from 382807e to 43f3431 Compare November 7, 2018 17:44
@lorentey
Copy link
Member Author

lorentey commented Nov 7, 2018

@swift-ci smoke test and merge

@swift-ci swift-ci merged commit 908ce05 into swiftlang:master Nov 7, 2018
@lorentey lorentey deleted the hasher-padding-redux2 branch November 20, 2018 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants