Skip to content

[stdlib] Fix removeLast(_:) performance for non-random-access collections #32599

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jul 1, 2020
Merged

[stdlib] Fix removeLast(_:) performance for non-random-access collections #32599

merged 3 commits into from
Jul 1, 2020

Conversation

natecook1000
Copy link
Member

This replaces the count comparison precondition with a limited index offset, which converts the method from O(n) to O(k). This follows up on #32451.

@natecook1000
Fix removeLast(_:) performance for non-random-access collections
ed9348d 
This replaces the `count` comparison precondition with a limited index
offset, which converts the method from O(n) to O(k).
@natecook1000
Copy link
Member Author

@swift-ci Please smoke benchmark

@swift-ci
Copy link
Contributor

Performance: -O

Regression OLD NEW DELTA RATIO
LessSubstringSubstring 38 42 +10.5% 0.90x
LessSubstringSubstringGenericComparable 38 42 +10.5% 0.90x
FlattenListFlatMap 3814 4168 +9.3% 0.92x (?)
Array2D 6672 7216 +8.2% 0.92x (?)
EqualSubstringSubstring 38 41 +7.9% 0.93x (?)
EqualSubstringSubstringGenericEquatable 38 41 +7.9% 0.93x (?)
EqualSubstringString 38 41 +7.9% 0.93x (?)
EqualStringSubstring 39 42 +7.7% 0.93x (?)
 
Improvement OLD NEW DELTA RATIO
StringComparison_slowerPrenormal 1650 1490 -9.7% 1.11x (?)
StringComparison_nonBMPSlowestPrenormal 1670 1520 -9.0% 1.10x (?)
RandomTree.insert.Unmanaged.fast 208 193 -7.2% 1.08x (?)
StringComparison_emoji 900 840 -6.7% 1.07x (?)

Code size: -O

Performance: -Osize

Regression OLD NEW DELTA RATIO
LessSubstringSubstring 38 42 +10.5% 0.90x
LessSubstringSubstringGenericComparable 38 42 +10.5% 0.90x
String.data.Medium 106 116 +9.4% 0.91x (?)
Array2D 6928 7520 +8.5% 0.92x
EqualStringSubstring 38 41 +7.9% 0.93x (?)
EqualSubstringSubstring 39 42 +7.7% 0.93x (?)
EqualSubstringSubstringGenericEquatable 39 42 +7.7% 0.93x
EqualSubstringString 39 42 +7.7% 0.93x (?)
 
Improvement OLD NEW DELTA RATIO
StringComparison_slowerPrenormal 1660 1520 -8.4% 1.09x (?)
StringComparison_nonBMPSlowestPrenormal 1720 1590 -7.6% 1.08x (?)
StringWalk 2720 2520 -7.4% 1.08x (?)
StrComplexWalk 5160 4800 -7.0% 1.07x (?)
DataAccessBytesMedium 101 94 -6.9% 1.07x (?)
RandomTree.insert.Unmanaged.fast 210 196 -6.7% 1.07x (?)

Code size: -Osize

Performance: -Onone

Improvement OLD NEW DELTA RATIO
ArrayAppendAsciiSubstring 47880 41616 -13.1% 1.15x (?)
ArrayAppendLatin1Substring 48528 42300 -12.8% 1.15x (?)
ObjectiveCBridgeStubToNSDateRef 4620 4200 -9.1% 1.10x (?)
StrComplexWalk 8230 7570 -8.0% 1.09x
CharIteration_chinese_unicodeScalars_Backwards 261280 240720 -7.9% 1.09x (?)
ArrayOfPOD 1111 1030 -7.3% 1.08x (?)

Code size: -swiftlibs

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

@natecook1000
Copy link
Member Author

@swift-ci Please smoke benchmark

@swift-ci
Copy link
Contributor

Performance: -O

Regression OLD NEW DELTA RATIO
LessSubstringSubstring 38 42 +10.5% 0.90x (?)
LessSubstringSubstringGenericComparable 38 42 +10.5% 0.90x (?)
Array2D 6672 7216 +8.2% 0.92x (?)
EqualSubstringSubstring 38 41 +7.9% 0.93x (?)
EqualSubstringSubstringGenericEquatable 38 41 +7.9% 0.93x (?)
EqualSubstringString 38 41 +7.9% 0.93x (?)
EqualStringSubstring 39 42 +7.7% 0.93x (?)
 
Improvement OLD NEW DELTA RATIO
SubstringRemoveLast1 151 0 -100.0% 151001.00x
StringComparison_slowerPrenormal 1650 1490 -9.7% 1.11x
StringComparison_nonBMPSlowestPrenormal 1670 1520 -9.0% 1.10x (?)
String.data.LargeUnicode 113 104 -8.0% 1.09x (?)
String.data.Medium 108 100 -7.4% 1.08x (?)
RandomTree.insert.Unmanaged.fast 208 194 -6.7% 1.07x (?)

Code size: -O

Improvement OLD NEW DELTA RATIO
Substring.o 18547 18355 -1.0% 1.01x

Performance: -Osize

Regression OLD NEW DELTA RATIO
FlattenListLoop 1632 2466 +51.1% 0.66x (?)
FlattenListFlatMap 4845 6601 +36.2% 0.73x (?)
LessSubstringSubstring 38 42 +10.5% 0.90x
LessSubstringSubstringGenericComparable 38 42 +10.5% 0.90x
Array2D 6928 7520 +8.5% 0.92x (?)
EqualStringSubstring 38 41 +7.9% 0.93x (?)
EqualSubstringSubstringGenericEquatable 39 42 +7.7% 0.93x (?)
EqualSubstringString 39 42 +7.7% 0.93x (?)
 
Improvement OLD NEW DELTA RATIO
SubstringRemoveLast1 154 0 -100.0% 154001.00x
StrComplexWalk 5240 4790 -8.6% 1.09x
StringComparison_slowerPrenormal 1650 1520 -7.9% 1.09x (?)
DataAccessBytesMedium 102 94 -7.8% 1.09x (?)
StringComparison_nonBMPSlowestPrenormal 1720 1590 -7.6% 1.08x (?)
StringWalk 2720 2520 -7.4% 1.08x
RandomTree.insert.Unmanaged.fast 209 195 -6.7% 1.07x (?)

Code size: -Osize

Improvement OLD NEW DELTA RATIO
Substring.o 18779 18585 -1.0% 1.01x

Performance: -Onone

Regression OLD NEW DELTA RATIO
StringToDataMedium 4400 4950 +12.5% 0.89x (?)
ObjectiveCBridgeFromNSArrayAnyObjectForced 7300 8100 +11.0% 0.90x (?)
EqualSubstringSubstring 45 49 +8.9% 0.92x
DictionaryBridgeToObjC_Access 868 944 +8.8% 0.92x (?)
 
Improvement OLD NEW DELTA RATIO
SubstringRemoveLast1 146 0 -100.0% 146001.00x
ArrayAppendAsciiSubstring 48996 40212 -17.9% 1.22x (?)
ArrayAppendLatin1Substring 49680 40896 -17.7% 1.21x (?)
ArrayAppendUTF16Substring 49032 40392 -17.6% 1.21x (?)
ObjectiveCBridgeStubToNSDateRef 4640 4240 -8.6% 1.09x (?)
StrComplexWalk 8230 7570 -8.0% 1.09x (?)
ArrayOfPOD 1112 1031 -7.3% 1.08x (?)

Code size: -swiftlibs

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

@natecook1000 natecook1000 requested a review from lorentey June 29, 2020 22:17
lorentey
lorentey reviewed Jun 30, 2020
View reviewed changes
@natecook1000 @lorentey
Update stdlib/public/core/RangeReplaceableCollection.swift
d6a9f66 
Co-authored-by: Karoy Lorentey <klorentey@apple.com>
@natecook1000
Copy link
Member Author

@swift-ci Please smoke test

@natecook1000 natecook1000 merged commit ba968d4 into swiftlang:master Jul 1, 2020
@natecook1000 natecook1000 deleted the nate/substring_removelast branch July 1, 2020 08:06
natecook1000 added a commit to natecook1000/swift that referenced this pull request Jul 1, 2020
@natecook1000
[stdlib] Fix removeLast(_:) performance for non-random-access collect…
69f7b4f 
…ions (swiftlang#32599)

This replaces the `count` comparison precondition with a limited index
offset, which converts the method from O(n) to O(k).
@natecook1000 natecook1000 mentioned this pull request Jul 1, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lorentey lorentey lorentey approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@natecook1000 @swift-ci @lorentey