Skip to content

[String] [Foundation] perf: UTF8 String -> Data #24215

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

ianpartridge
Copy link
Contributor

@ianpartridge ianpartridge commented Apr 23, 2019

Currently, str.data(using:allowLossyConversion:) always bridges via
NSString.

As String is now natively UTF-8 we can fastpath this conversion in the
case where the user requests UTF-8 encoding.

A benchmark for this was previously added in #22648.

Currently, `str.data(using:allowLossyConversion)` always bridges via
`NSString`.

As `String` is now natively UTF8 we can fastpath this conversion in the
case where the user requests UTF8 encoding.

A benchmark for this was previously added in swiftlang#22648.
@ianpartridge
Copy link
Contributor Author

@swift-ci please benchmark

@swift-ci
Copy link
Contributor

Performance: -O

TEST OLD NEW DELTA RATIO
Improvement
String.data.Small 341 31 -90.9% 11.00x
String.data.LargeUnicode 471 87 -81.5% 5.41x
String.data.Medium 418 84 -79.9% 4.98x
String.data.Empty 83 29 -65.1% 2.86x

Performance: -Osize

TEST OLD NEW DELTA RATIO
Improvement
String.data.Small 359 32 -91.1% 11.22x
String.data.LargeUnicode 496 91 -81.7% 5.45x
String.data.Medium 434 87 -80.0% 4.99x
String.data.Empty 84 30 -64.3% 2.80x
DataAppendDataLargeToLarge 50400 37600 -25.4% 1.34x (?)

Performance: -Onone

TEST OLD NEW DELTA RATIO
Improvement
String.data.Small 363 36 -90.1% 10.08x
String.data.LargeUnicode 504 99 -80.4% 5.09x
String.data.Medium 426 90 -78.9% 4.73x
String.data.Empty 91 33 -63.7% 2.76x
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

@ianpartridge
Copy link
Contributor Author

10x for small strings, niiiiiiiiiiice 😎

@itaiferber @milseman please review.

@itaiferber itaiferber self-requested a review April 23, 2019 15:53
@milseman
Copy link
Member

Sweet!

CC @Catfish-Man

@milseman
Copy link
Member

Also, let's cherry-pick this onto the 5.1 branch

@ianpartridge
Copy link
Contributor Author

@swift-ci please smoke test

@ianpartridge
Copy link
Contributor Author

Please merge when you're ready - thanks.

@itaiferber itaiferber merged commit a6fbc40 into swiftlang:master Apr 24, 2019
@ianpartridge ianpartridge deleted the string-data-conversion-perf branch April 24, 2019 16:29
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.

4 participants