Skip to content

[stdlib] Add 256 bits of extra state to Hasher #20202

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 2 commits into from
Nov 1, 2018

Conversation

lorentey
Copy link
Member

@lorentey lorentey commented Oct 31, 2018

This is the ugly part of #20185, without anysome of the ABI cleanups.

The original PR also removed the _HasherCore protocol, but that leads to code size and performance regressions due to overenthusiastic inlining. (See #20198)

@lorentey
Copy link
Member Author

@swift-ci please benchmark

@swift-ci
Copy link
Contributor

Build comment file:

Code size: -O

TEST OLD NEW DELTA RATIO
Regression
DictionarySwap.o 27430 27894 +1.7% 0.98x
DictTest4Legacy.o 26936 27384 +1.7% 0.98x
DictTest4.o 26046 26430 +1.5% 0.99x

Code size: -Osize

TEST OLD NEW DELTA RATIO
Regression
DictTest4.o 21199 21535 +1.6% 0.98x
DictTest4Legacy.o 22585 22905 +1.4% 0.99x

Performance: -Onone

TEST OLD NEW DELTA RATIO
Improvement
Histogram 6844 5753 -15.9% 1.19x
RGBHistogram 24335 21601 -11.2% 1.13x
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: 16 GB

@lorentey
Copy link
Member Author

@swift-ci please smoke test and merge

Also, eliminate the (unused) SipHash24 implementation and nest most of Hasher’s internals under Hasher itself.
@lorentey lorentey force-pushed the hasher-padding-redux3 branch from ecef750 to 3ae170c Compare November 1, 2018 00:46
@lorentey
Copy link
Member Author

lorentey commented Nov 1, 2018

@swift-ci please smoke test and merge

@lorentey lorentey merged commit dce36dc into swiftlang:master Nov 1, 2018
@lorentey lorentey deleted the hasher-padding-redux3 branch November 1, 2018 01:30
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