Skip to content

Conversation

@lorentey
Copy link
Member

@lorentey lorentey commented Nov 1, 2018

The code to compare character values can be huge; it doesn’t seem a good idea to forcibly inline it. The @inline(__always) here can make unrelated modifications to Set's code trigger a ballooning of the size of specialized Set<Character> operations to as much as 10x their usual size. (E.g., insert grows from 1.5kB to 15kB.) It also causes ~20% slowdowns in some benchmarks.

The code size bloat doesn’t always happen; I guess that _fastPaths inside String.== cause unintuitive inlining decisions in callers.

The code to compare character values can be huge; it doesn’t seem a good idea to forcibly inline it. The @inline(__always) here can make specialized Set<Character> operations balloon 10x their current size with unrelated modifications in Set.

The code size bloat doesn’t always happen; I *guess* that _fastPaths inside String.== cause unintuitive inlining decisions in callers.
@lorentey
Copy link
Member Author

lorentey commented Nov 1, 2018

See #20198 for an example of a change that causes code size to balloon.

@lorentey
Copy link
Member Author

lorentey commented Nov 1, 2018

@swift-ci please test

@lorentey
Copy link
Member Author

lorentey commented Nov 1, 2018

@swift-ci please benchmark

@lorentey lorentey requested a review from milseman November 1, 2018 12:33
@swift-ci
Copy link
Contributor

swift-ci commented Nov 1, 2018

Build comment file:

Performance: -O

TEST OLD NEW DELTA RATIO
Regression
SuperChars 19164 21812 +13.8% 0.88x
Improvement
CountAlgoString 3409 2950 -13.5% 1.16x
StringHasPrefixUnicode 116631 103228 -11.5% 1.13x
StringMatch 8369 7478 -10.6% 1.12x
StringComparison_abnormal 853 782 -8.3% 1.09x
StringHashing_abnormal 1431 1317 -8.0% 1.09x
CStringLongAscii 3525 3277 -7.0% 1.08x

Code size: -O

TEST OLD NEW DELTA RATIO
Regression
Chars.o 13172 18868 +43.2% 0.70x
SuperChars.o 13702 16534 +20.7% 0.83x
Improvement
StringMatch.o 41607 16839 -59.5% 2.47x
ReduceInto.o 52841 24191 -54.2% 2.18x
CountAlgo.o 45782 22764 -50.3% 2.01x
StringRemoveDupes.o 33546 17109 -49.0% 1.96x
WordCount.o 107570 62192 -42.2% 1.73x
CSVParsing.o 55635 47699 -14.3% 1.17x
DriverUtils.o 188783 170183 -9.9% 1.11x
RomanNumbers.o 15253 14181 -7.0% 1.08x
Combos.o 18237 17341 -4.9% 1.05x
Substring.o 31401 30841 -1.8% 1.02x
StringComparison.o 55468 54908 -1.0% 1.01x

Performance: -Osize

TEST OLD NEW DELTA RATIO
Regression
SuperChars 18962 23515 +24.0% 0.81x
UTF8Decode_InitFromBytes_ascii 511 607 +18.8% 0.84x
Chars2 2925 3292 +12.5% 0.89x
Improvement
CountAlgoString 3544 3067 -13.5% 1.16x
StringHasPrefixUnicode 116341 103170 -11.3% 1.13x
StringMatch 8989 8179 -9.0% 1.10x
StringComparison_abnormal 833 768 -7.8% 1.08x
StringHashing_abnormal 1432 1322 -7.7% 1.08x (?)
UTF8Decode_InitFromBytes 1250 1162 -7.0% 1.08x (?)

Code size: -Osize

TEST OLD NEW DELTA RATIO
Improvement
StringMatch.o 33575 5487 -83.7% 6.12x
StringRemoveDupes.o 28850 7733 -73.2% 3.73x
SuperChars.o 11743 3247 -72.3% 3.62x
ReduceInto.o 46096 13562 -70.6% 3.40x
CountAlgo.o 39645 12896 -67.5% 3.07x
RomanNumbers.o 13830 5246 -62.1% 2.64x
Chars.o 11469 4845 -57.8% 2.37x
Combos.o 16581 8205 -50.5% 2.02x
WordCount.o 93562 50349 -46.2% 1.86x
Substring.o 27619 19425 -29.7% 1.42x
CSVParsing.o 50483 36203 -28.3% 1.39x
StringComparison.o 49996 41716 -16.6% 1.20x
DriverUtils.o 161659 139075 -14.0% 1.16x

Performance: -Onone

TEST OLD NEW DELTA RATIO
Improvement
ArrayOfGenericPOD2 1237 1066 -13.8% 1.16x (?)
StringHasPrefixUnicode 119635 106923 -10.6% 1.12x
ArrayOfPOD 860 783 -9.0% 1.10x
CStringLongAscii 3615 3367 -6.9% 1.07x

Code size: Swift libraries

TEST OLD NEW DELTA RATIO
Improvement
libswiftCore.dylib 3813376 3743744 -1.8% 1.02x
libswiftStdlibUnittest.dylib 409600 405504 -1.0% 1.01x
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 lorentey merged commit eea96a3 into swiftlang:master Nov 3, 2018
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