Skip to content
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

[stdlib] Demote internal preconditions to invariant checks #21132

Closed
wants to merge 2 commits into from

Conversation

milseman
Copy link
Contributor

@milseman milseman commented Dec 7, 2018

No description provided.

@milseman
Copy link
Contributor Author

milseman commented Dec 7, 2018

@swift-ci please benchmark

@swift-ci
Copy link
Collaborator

swift-ci commented Dec 8, 2018

Build comment file:

Build failed before running benchmark.


@milseman
Copy link
Contributor Author

milseman commented Dec 8, 2018

@swift-ci please benchmark

@milseman
Copy link
Contributor Author

milseman commented Dec 8, 2018

@swift-ci
Copy link
Collaborator

swift-ci commented Dec 8, 2018

Build comment file:

Performance: -O

TEST OLD NEW DELTA RATIO
Regression
ArrayAppendAscii 3128 3774 +20.7% 0.83x (?)
ArrayAppendUTF16 3128 3740 +19.6% 0.84x
ArrayAppendLatin1 3230 3808 +17.9% 0.85x
NSStringConversion 544 596 +9.6% 0.91x (?)
Improvement
IterateData 1563 1399 -10.5% 1.12x
FrequenciesUsingReduceInto 932 870 -6.7% 1.07x (?)

Code size: -O

TEST OLD NEW DELTA RATIO
Regression
StringEdits.o 11967 12175 +1.7% 0.98x
Improvement
StringMatch.o 4606 4525 -1.8% 1.02x

Performance: -Osize

TEST OLD NEW DELTA RATIO
Improvement
IterateData 1547 1356 -12.3% 1.14x
CharIteration_russian_unicodeScalars_Backwards 5721 5097 -10.9% 1.12x

Code size: -Osize

TEST OLD NEW DELTA RATIO
Improvement
StringMatch.o 4649 4513 -2.9% 1.03x
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
--------------

@milseman
Copy link
Contributor Author

milseman commented Dec 8, 2018

@swift-ci please benchmark

@milseman
Copy link
Contributor Author

milseman commented Dec 8, 2018

Hmm, I had around 10% locally on StringWordBuilder, but not sure why it doesn't repro.

@swift-ci
Copy link
Collaborator

swift-ci commented Dec 8, 2018

Build comment file:

Performance: -O

TEST OLD NEW DELTA RATIO
Regression
ArrayAppendAscii 3094 3774 +22.0% 0.82x
ArrayAppendUTF16 3094 3774 +22.0% 0.82x
ArrayAppendLatin1 3196 3842 +20.2% 0.83x
UTF8Decode_InitFromBytes 1181 1337 +13.2% 0.88x (?)
SortStringsUnicode 3306 3613 +9.3% 0.92x
Improvement
IterateData 1564 1400 -10.5% 1.12x
CountAlgoString 1890 1760 -6.9% 1.07x

Code size: -O

TEST OLD NEW DELTA RATIO
Regression
StringEdits.o 11967 12175 +1.7% 0.98x
Improvement
StringMatch.o 4606 4525 -1.8% 1.02x

Performance: -Osize

TEST OLD NEW DELTA RATIO
Regression
Breadcrumbs.CopyUTF16CodeUnits.Mixed 55 66 +20.0% 0.83x
SortStringsUnicode 3287 3596 +9.4% 0.91x
CStringLongAscii 434 471 +8.5% 0.92x
Improvement
IterateData 1536 1363 -11.3% 1.13x
CharIteration_russian_unicodeScalars_Backwards 5707 5089 -10.8% 1.12x

Code size: -Osize

TEST OLD NEW DELTA RATIO
Improvement
StringMatch.o 4649 4513 -2.9% 1.03x

Performance: -Onone

TEST OLD NEW DELTA RATIO
Regression
ArrayOfPOD 778 859 +10.4% 0.91x (?)
Improvement
Breadcrumbs.MutatedIdxToUTF16.ASCII 31 22 -29.0% 1.41x
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
--------------

@milseman milseman changed the title [stdlib] Unchecked versions of common UnsafePointer operations [stdlib] Demote internal precondition checks to invariant checks Dec 10, 2018
@milseman milseman changed the title [stdlib] Demote internal precondition checks to invariant checks [stdlib] Demote internal preconditions to invariant checks Dec 10, 2018
@milseman
Copy link
Contributor Author

@swift-ci please benchmark

@milseman
Copy link
Contributor Author

@swift-ci
Copy link
Collaborator

Build comment file:

Performance: -O

TEST OLD NEW DELTA RATIO
Regression
CharacterLiteralsSmall 348 437 +25.6% 0.80x
ArrayAppendLatin1 3196 3842 +20.2% 0.83x
ArrayAppendUTF16 3128 3740 +19.6% 0.84x
Ackermann 225 250 +11.1% 0.90x
MapReduceLazyCollectionShort 31 34 +9.7% 0.91x (?)
SubstringComparable 12 13 +8.3% 0.92x
Improvement
AngryPhonebook 2905 2303 -20.7% 1.26x
SortAdjacentIntPyramids 1204 997 -17.2% 1.21x
Breadcrumbs.CopyUTF16CodeUnits.Mixed 66 56 -15.2% 1.18x
StringAdder 491 431 -12.2% 1.14x
FatCompactMap 1400 1230 -12.1% 1.14x
IterateData 1565 1398 -10.7% 1.12x
StringBuilder 367 330 -10.1% 1.11x
StringUTF16Builder 392 354 -9.7% 1.11x
StringBuilderSmallReservingCapacity 377 341 -9.5% 1.11x
FloatingPointPrinting_Float_interpolated 40286 37025 -8.1% 1.09x
FloatingPointPrinting_Float_description_uniform 5644 5232 -7.3% 1.08x

Code size: -O

TEST OLD NEW DELTA RATIO
Regression
SortStrings.o 28096 89006 +216.8% 0.32x
SuperChars.o 1124 2457 +118.6% 0.46x
AngryPhonebook.o 1035 1922 +85.7% 0.54x
DictTest.o 19206 34214 +78.1% 0.56x
Chars.o 1619 2696 +66.5% 0.60x
Phonebook.o 11963 14299 +19.5% 0.84x
CString.o 8482 9986 +17.7% 0.85x
Hash.o 34009 35950 +5.7% 0.95x
PrefixWhile.o 22878 23918 +4.5% 0.96x
DropWhile.o 22964 24004 +4.5% 0.96x
TestsUtils.o 24731 25850 +4.5% 0.96x
Prefix.o 24329 25337 +4.1% 0.96x
SequenceAlgos.o 20795 21620 +4.0% 0.96x
DropFirst.o 25044 26004 +3.8% 0.96x
Suffix.o 25873 26849 +3.8% 0.96x
DropLast.o 25931 26891 +3.7% 0.96x
TwoSum.o 5524 5726 +3.7% 0.96x
StringTests.o 8094 8380 +3.5% 0.97x
ObjectiveCBridging.o 43320 44744 +3.3% 0.97x
MapReduce.o 32947 34003 +3.2% 0.97x
Exclusivity.o 4507 4635 +2.8% 0.97x
DictionarySwap.o 27750 28518 +2.8% 0.97x
StringEdits.o 11967 12287 +2.7% 0.97x
ReduceInto.o 18201 18681 +2.6% 0.97x
StringMatch.o 4606 4706 +2.2% 0.98x
StringEnum.o 12037 12277 +2.0% 0.98x
ObjectAllocation.o 4139 4219 +1.9% 0.98x
BinaryFloatingPointProperties.o 7612 7740 +1.7% 0.98x
CSVParsing.o 30257 30761 +1.7% 0.98x
DictionaryCopy.o 7773 7901 +1.6% 0.98x
ObjectiveCBridgingStubs.o 19363 19674 +1.6% 0.98x
ChainedFilterMap.o 3374 3422 +1.4% 0.99x
DictionarySubscriptDefault.o 29441 29809 +1.2% 0.99x
PopFront.o 5213 5276 +1.2% 0.99x
LazyFilter.o 11193 11321 +1.1% 0.99x
SetTests.o 64415 65119 +1.1% 0.99x
Improvement
DictionaryOfAnyHashableStrings.o 11101 10325 -7.0% 1.08x
DriverUtils.o 151237 142859 -5.5% 1.06x
CharacterLiteralsSmall.o 1325 1252 -5.5% 1.06x
CharacterLiteralsLarge.o 883 835 -5.4% 1.06x
RC4.o 4115 3992 -3.0% 1.03x
StaticArray.o 13948 13532 -3.0% 1.03x
SevenBoom.o 1642 1601 -2.5% 1.03x
RangeIteration.o 1700 1665 -2.1% 1.02x
Ackermann.o 1852 1814 -2.1% 1.02x
ByteSwap.o 1652 1619 -2.0% 1.02x
NSDictionaryCastToSwift.o 1673 1641 -1.9% 1.02x
PointerArithmetics.o 1759 1727 -1.8% 1.02x
DeadArray.o 1892 1858 -1.8% 1.02x
Integrate.o 2456 2421 -1.4% 1.01x
XorLoop.o 2028 2000 -1.4% 1.01x
Calculator.o 2708 2671 -1.4% 1.01x
Fibonacci.o 1612 1590 -1.4% 1.01x
ArraySubscript.o 3892 3839 -1.4% 1.01x
ArrayLiteral.o 3143 3104 -1.2% 1.01x
BitCount.o 1876 1853 -1.2% 1.01x
RomanNumbers.o 5375 5311 -1.2% 1.01x
StrComplexWalk.o 2785 2756 -1.0% 1.01x
OpenClose.o 3282 3248 -1.0% 1.01x

Performance: -Osize

TEST OLD NEW DELTA RATIO
Regression
ObjectiveCBridgeStubToNSStringRef 109 132 +21.1% 0.83x (?)
CharacterLiteralsSmall 345 410 +18.8% 0.84x
Improvement
AngryPhonebook 2905 2275 -21.7% 1.28x
Breadcrumbs.CopyUTF16CodeUnits.Mixed 65 55 -15.4% 1.18x
IterateData 1566 1358 -13.3% 1.15x
StringBuilder 367 321 -12.5% 1.14x
StringUTF16Builder 392 345 -12.0% 1.14x
StringBuilderSmallReservingCapacity 377 332 -11.9% 1.14x
CharIteration_russian_unicodeScalars_Backwards 5814 5125 -11.9% 1.13x
StringAdder 467 416 -10.9% 1.12x
CharacterLiteralsLarge 111 100 -9.9% 1.11x
FloatingPointPrinting_Float_description_uniform 5688 5295 -6.9% 1.07x
SubstringEquatable 803 748 -6.8% 1.07x

Code size: -Osize

TEST OLD NEW DELTA RATIO
Regression
SortStrings.o 28687 89245 +211.1% 0.32x
SuperChars.o 1246 2542 +104.0% 0.49x
DictTest.o 17233 32513 +88.7% 0.53x
AngryPhonebook.o 1144 2024 +76.9% 0.57x
Chars.o 1741 2765 +58.8% 0.63x
Phonebook.o 12380 14716 +18.9% 0.84x
CString.o 8314 9850 +18.5% 0.84x
StringMatch.o 4649 4841 +4.1% 0.96x
ObjectiveCBridgingStubs.o 18475 18731 +1.4% 0.99x
Hash.o 20367 20639 +1.3% 0.99x
DictTest4Legacy.o 23944 24232 +1.2% 0.99x
SortLettersInPlace.o 8950 9054 +1.2% 0.99x
Improvement
DictionaryOfAnyHashableStrings.o 10757 10125 -5.9% 1.06x
CharacterLiteralsSmall.o 1428 1348 -5.6% 1.06x
CharacterLiteralsLarge.o 948 900 -5.1% 1.05x
TestsUtils.o 19019 18378 -3.4% 1.03x
DriverUtils.o 131797 127879 -3.0% 1.03x
ReversedCollections.o 11842 11706 -1.1% 1.01x
DictTest3.o 21233 21009 -1.1% 1.01x

Performance: -Onone

TEST OLD NEW DELTA RATIO
Improvement
CharacterLiteralsLarge 597 511 -14.4% 1.17x

Code size: -swiftlibs

TEST OLD NEW DELTA RATIO
Regression
libswiftCore.dylib 3411968 3489792 +2.3% 0.98x
Improvement
libswiftStdlibUnittest.dylib 380928 376832 -1.1% 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
--------------

@milseman
Copy link
Contributor Author

The regressions seem to be due to a bug in the SIL Optimizer stopping it from constant-folding small strings. Holding off until we can learn more.

rdar://problem/46606173

@milseman
Copy link
Contributor Author

The optimizer fix that re-enabled constant folding: #21192

@milseman
Copy link
Contributor Author

@swift-ci please benchmark

@swift-ci
Copy link
Collaborator

Build comment file:

Performance: -O

TEST OLD NEW DELTA RATIO
Regression
ArrayAppendAscii 3060 3672 +20.0% 0.83x
ArrayAppendUTF16 3060 3672 +20.0% 0.83x
ArrayAppendLatin1 3162 3774 +19.4% 0.84x
IterateData 1401 1559 +11.3% 0.90x
Improvement
AngryPhonebook 2926 2261 -22.7% 1.29x
StringUTF16Builder 413 352 -14.8% 1.17x
StringBuilder 377 328 -13.0% 1.15x
StringBuilderSmallReservingCapacity 388 340 -12.4% 1.14x
StringAdder 480 424 -11.7% 1.13x
RemoveWhereFilterString 410 379 -7.6% 1.08x

Code size: -O

TEST OLD NEW DELTA RATIO
Regression
StringEdits.o 11967 12175 +1.7% 0.98x
Improvement
StringMatch.o 4606 4525 -1.8% 1.02x

Performance: -Osize

TEST OLD NEW DELTA RATIO
Regression
Breadcrumbs.CopyUTF16CodeUnits.Mixed 55 65 +18.2% 0.85x
IterateData 1403 1562 +11.3% 0.90x
Improvement
AngryPhonebook 2926 2289 -21.8% 1.28x
StringAdder 477 423 -11.3% 1.13x
CharIteration_russian_unicodeScalars_Backwards 5721 5080 -11.2% 1.13x
FloatingPointPrinting_Double_interpolated 60171 53674 -10.8% 1.12x
StringBuilder 365 328 -10.1% 1.11x
StringUTF16Builder 391 352 -10.0% 1.11x
StringBuilderSmallReservingCapacity 375 340 -9.3% 1.10x
StringInterpolationManySmallSegments 12025 11058 -8.0% 1.09x
RemoveWhereFilterString 402 373 -7.2% 1.08x

Code size: -Osize

TEST OLD NEW DELTA RATIO
Improvement
StringMatch.o 4649 4513 -2.9% 1.03x
ReversedCollections.o 11842 11706 -1.1% 1.01x

Performance: -Onone

TEST OLD NEW DELTA RATIO
Improvement
CharacterLiteralsLarge 600 502 -16.3% 1.20x
AngryPhonebook 5845 5348 -8.5% 1.09x

Code size: -swiftlibs

TEST OLD NEW DELTA RATIO
Regression
libswiftSwiftPrivate.dylib 40960 45056 +10.0% 0.91x
libswiftCore.dylib 3407872 3485696 +2.3% 0.98x
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
--------------

@atrick
Copy link
Member

atrick commented Feb 5, 2019

I wrote in inlining heuristic fix for this silliness months ago, but that itself has regressions so it's a regression deadlock. Hopefully I'll get back to that very soon though.

@milseman
Copy link
Contributor Author

milseman commented Feb 6, 2019

This has both nice improvements and regressions. Maybe the combination of the two would help?

@atrick
Copy link
Member

atrick commented Feb 6, 2019

Feel free to file an SR where we can list benchmarks that have regressed because of inlining noise triggered by removing branches and similar changes along with the old run times. Then we can come back after these improvements/bug fixes land and if they haven't recovered figure out why they're so noisy.

@shahmishal
Copy link
Member

Please update the base branch to main by Oct 5th otherwise the pull request will be closed automatically.

  • How to change the base branch: (Link)
  • More detail about the branch update: (Link)

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.

None yet

4 participants