Skip to content

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