Skip to content

[stdlib] Use ContiguousArray internally in Sequence dropLast, prefix, suffix #21658

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
merged 2 commits into from
Jan 8, 2019

Conversation

palimondo
Copy link
Contributor

@palimondo palimondo commented Jan 5, 2019

Following the implementation template used in methods map and _filter, use the ContiguousArray internally in default implementations of dropLast, prefix and suffix. This is converted into an Array on return in a constant time. In addition to increased consistency of the internal implementation, this noticeably improves the performance of these methods for UnfoldSequence benchmark variants in the unoptimized build.

This PR is based on discussion in #20221 (review) and was split off from an experiment in #20758.

Nano-optimization:
Following the template used in methods `map` and `_filter`, internally use ContiguousArray before converting to an Array on return.
@palimondo
Copy link
Contributor Author

@swift-ci please benchmark

@palimondo
Copy link
Contributor Author

@swift-ci please smoke test

@swift-ci
Copy link
Contributor

swift-ci commented Jan 5, 2019

Build comment file:

Performance: -O

TEST OLD NEW DELTA RATIO
Improvement
PrefixWhileAnySeqCntRange 362 326 -9.9% 1.11x

Code size: -O

TEST OLD NEW DELTA RATIO
Improvement
PrefixWhile.o 22878 21422 -6.4% 1.07x

Performance: -Osize

TEST OLD NEW DELTA RATIO
Regression
PrefixWhileAnySeqCntRangeLazy 159 176 +10.7% 0.90x
Improvement
PrefixWhileAnyCollectionLazy 176 160 -9.1% 1.10x
DropLastCountableRangeLazy 12 11 -8.3% 1.09x
PrefixWhileAnyCollection 253 235 -7.1% 1.08x

Code size: -Osize

TEST OLD NEW DELTA RATIO
Improvement
PrefixWhile.o 21366 20118 -5.8% 1.06x

Performance: -Onone

TEST OLD NEW DELTA RATIO
Improvement
DropLastAnySequenceLazy 28499 18994 -33.4% 1.50x
DropLastSequence 27964 18718 -33.1% 1.49x
DropLastAnySequence 27937 18840 -32.6% 1.48x
DropLastSequenceLazy 27968 18938 -32.3% 1.48x
SuffixAnySequenceLazy 23969 18597 -22.4% 1.29x
SuffixSequence 23079 17963 -22.2% 1.28x
SuffixSequenceLazy 22847 17792 -22.1% 1.28x
SuffixAnySequence 22865 17848 -21.9% 1.28x
PrefixWhileAnySequence 33375 27697 -17.0% 1.21x
DropLastAnySeqCRangeIterLazy 63657 53994 -15.2% 1.18x
DropLastAnySeqCRangeIter 62899 53509 -14.9% 1.18x
DropLastAnySeqCntRange 62850 53518 -14.8% 1.17x
PrefixWhileSequence 33147 29661 -10.5% 1.12x
DropLastAnySeqCntRangeLazy 63577 57544 -9.5% 1.10x
SuffixAnySeqCntRangeLazy 58688 54678 -6.8% 1.07x
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
--------------

@palimondo
Copy link
Contributor Author

@airspeedswift @dabrahams Please review 🙏

Copy link
Contributor

@dabrahams dabrahams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@palimondo
Copy link
Contributor Author

palimondo commented Jan 7, 2019

@dabrahams Should I merge this or wait for @airspeedswift to chime in?

Edit: I'll assume silence is a weak consent in this case. There is still the revert ability… 😇

@palimondo palimondo merged commit 66bc166 into swiftlang:master Jan 8, 2019
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.

3 participants