Skip to content

[SR-12648] Fixes filter function of RangeReplaceableCollection. #31911

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

Conversation

valeriyvan
Copy link
Contributor

@valeriyvan valeriyvan commented May 20, 2020

Fixes filter function of RangeReplaceableCollection.

Resolves SR-12648.

@valeriyvan valeriyvan changed the title Fixes filter function of RangeReplaceableCollection. [SR-12648] Fixes filter function of RangeReplaceableCollection. May 20, 2020
@theblixguy theblixguy requested a review from airspeedswift May 20, 2020 13:12
@gribozavr
Copy link
Contributor

@swift-ci Please test

@gribozavr
Copy link
Contributor

@swift-ci Please benchmark

@gribozavr
Copy link
Contributor

LGTM, but let's see what benchmarks say.

@swift-ci
Copy link
Contributor

Performance: -O

Regression OLD NEW DELTA RATIO
Dictionary4 152 234 +53.9% 0.65x
Dictionary4OfObjects 185 248 +34.1% 0.75x
Set.subtracting.Empty.Box 8 9 +12.5% 0.89x (?)
Dictionary3 139 152 +9.4% 0.91x (?)
 
Improvement OLD NEW DELTA RATIO
RemoveWhereFilterInts 26 23 -11.5% 1.13x (?)
RemoveWhereFilterString 248 223 -10.1% 1.11x (?)
OpenClose 63 58 -7.9% 1.09x (?)
Calculator 156 145 -7.1% 1.08x (?)

Code size: -O

Improvement OLD NEW DELTA RATIO
RemoveWhere.o 16757 16059 -4.2% 1.04x

Performance: -Osize

Regression OLD NEW DELTA RATIO
Dictionary4 159 252 +58.5% 0.63x
Dictionary4OfObjects 229 325 +41.9% 0.70x
Data.hash.Medium 30 33 +10.0% 0.91x (?)
PrefixWhileAnySeqCntRange 185 202 +9.2% 0.92x (?)
MapReduceString 48 52 +8.3% 0.92x (?)
 
Improvement OLD NEW DELTA RATIO
ObjectiveCBridgeStubToNSStringRef 74 69 -6.8% 1.07x (?)

Code size: -Osize

Improvement OLD NEW DELTA RATIO
RemoveWhere.o 16171 15337 -5.2% 1.05x

Performance: -Onone

Regression OLD NEW DELTA RATIO
CharacterLiteralsLarge 335 382 +14.0% 0.88x
StringWalk 2640 2920 +10.6% 0.90x (?)
Data.hash.Medium 36 39 +8.3% 0.92x (?)
ObjectiveCBridgeStubToNSDate2 380 410 +7.9% 0.93x (?)
 
Improvement OLD NEW DELTA RATIO
RemoveWhereFilterString 379 315 -16.9% 1.20x (?)

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 mini
  Model Identifier: Macmini8,1
  Processor Name: 6-Core Intel Core i7
  Processor Speed: 3.2 GHz
  Number of Processors: 1
  Total Number of Cores: 6
  L2 Cache (per Core): 256 KB
  L3 Cache: 12 MB
  Memory: 64 GB

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - a0b6517

@gribozavr
Copy link
Contributor

@swift-ci Please test Linux

@gribozavr
Copy link
Contributor

@swift-ci Please benchmark

@airspeedswift
Copy link
Member

Thanks! And thanks for spotting, @jrose-apple!

@swift-ci
Copy link
Contributor

Performance: -O

Regression OLD NEW DELTA RATIO
Dictionary4 152 240 +57.9% 0.63x
Dictionary4OfObjects 185 250 +35.1% 0.74x
FlattenListLoop 2873 3194 +11.2% 0.90x (?)
Data.hash.Medium 30 33 +10.0% 0.91x (?)
Dictionary3 140 153 +9.3% 0.92x (?)
 
Improvement OLD NEW DELTA RATIO
RemoveWhereFilterInts 26 23 -11.5% 1.13x (?)
RemoveWhereFilterString 248 225 -9.3% 1.10x (?)
StringToDataEmpty 550 500 -9.1% 1.10x (?)
StringBuilder 226 207 -8.4% 1.09x (?)
StringUTF16Builder 240 220 -8.3% 1.09x (?)
OpenClose 63 58 -7.9% 1.09x (?)
UTF8Decode_InitFromData_ascii 190 176 -7.4% 1.08x (?)

Code size: -O

Improvement OLD NEW DELTA RATIO
RemoveWhere.o 16757 16059 -4.2% 1.04x

Performance: -Osize

Regression OLD NEW DELTA RATIO
Dictionary4 159 252 +58.5% 0.63x
Dictionary4OfObjects 228 322 +41.2% 0.71x
Data.hash.Medium 30 33 +10.0% 0.91x (?)
MapReduceString 48 52 +8.3% 0.92x (?)
PrefixWhileAnySeqCRangeIter 185 199 +7.6% 0.93x (?)
 
Improvement OLD NEW DELTA RATIO
RemoveWhereMoveInts 21 18 -14.3% 1.17x (?)

Code size: -Osize

Improvement OLD NEW DELTA RATIO
RemoveWhere.o 16171 15337 -5.2% 1.05x

Performance: -Onone

Regression OLD NEW DELTA RATIO
CharacterLiteralsLarge 337 381 +13.1% 0.88x (?)
Data.hash.Medium 36 39 +8.3% 0.92x (?)
 
Improvement OLD NEW DELTA RATIO
RemoveWhereFilterString 379 315 -16.9% 1.20x

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 mini
  Model Identifier: Macmini8,1
  Processor Name: 6-Core Intel Core i7
  Processor Speed: 3.2 GHz
  Number of Processors: 1
  Total Number of Cores: 6
  L2 Cache (per Core): 256 KB
  L3 Cache: 12 MB
  Memory: 64 GB

@CodaFi
Copy link
Contributor

CodaFi commented May 28, 2020

@CodaFi CodaFi merged commit b78bd2e into swiftlang:master May 28, 2020
@CodaFi
Copy link
Contributor

CodaFi commented May 28, 2020

Please don't forget to update the SR>

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.

5 participants