Skip to content

Add benchmarks for creating Strings from ASCII #27665

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 1 commit into from
Oct 14, 2019

Conversation

Catfish-Man
Copy link
Contributor

No description provided.

@Catfish-Man Catfish-Man self-assigned this Oct 14, 2019
@Catfish-Man
Copy link
Contributor Author

@swift-ci please smoke benchmark

@swift-ci
Copy link
Contributor

Performance: -O

Regression OLD NEW DELTA RATIO
ArrayAppendOptionals 1350 2480 +83.7% 0.54x (?)
Breadcrumbs.IdxToUTF16Range.longASCII 34 38 +11.8% 0.89x
 
Improvement OLD NEW DELTA RATIO
Breadcrumbs.UTF16ToIdxRange.longASCII 32 28 -12.5% 1.14x
ObjectiveCBridgeStubFromNSDateRef 4210 3800 -9.7% 1.11x (?)
 
Added MIN MAX MEAN MAX_RSS
UTF8Decode_InitDecoding_ascii_as_ascii 154655 155776 155255
UTF8Decode_InitFromBytes_ascii_as_ascii 493 493 493
UTF8Decode_InitFromData_ascii_as_ascii 742 743 743

Code size: -O

Regression OLD NEW DELTA RATIO
UTF8Decode.o 11695 13400 +14.6% 0.87x

Performance: -Osize

Added MIN MAX MEAN MAX_RSS
UTF8Decode_InitDecoding_ascii_as_ascii 155004 157434 155858
UTF8Decode_InitFromBytes_ascii_as_ascii 643 645 644
UTF8Decode_InitFromData_ascii_as_ascii 817 829 822

Code size: -Osize

Regression OLD NEW DELTA RATIO
UTF8Decode.o 10391 11824 +13.8% 0.88x

Performance: -Onone

Added MIN MAX MEAN MAX_RSS
UTF8Decode_InitDecoding_ascii_as_ascii 156050 156692 156424
UTF8Decode_InitFromBytes_ascii_as_ascii 572 572 572
UTF8Decode_InitFromData_ascii_as_ascii 837 841 839

Code size: -swiftlibs

Benchmark Check Report
⛔️🔤 UTF8Decode_InitFromData_ascii_as_ascii name doesn`t conform to benchmark naming convention.
See http://bit.ly/BenchmarkNaming
⛔️🔤 UTF8Decode_InitDecoding_ascii_as_ascii name doesn`t conform to benchmark naming convention.
See http://bit.ly/BenchmarkNaming
⛔️⏱ UTF8Decode_InitDecoding_ascii_as_ascii execution took at least 154614 μs.
Decrease the workload of UTF8Decode_InitDecoding_ascii_as_ascii by a factor of 256 (1000), to be less than 1000 μs.
⛔️🔤 UTF8Decode_InitFromBytes_ascii_as_ascii name doesn`t conform to benchmark naming convention.
See http://bit.ly/BenchmarkNaming
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

@Catfish-Man
Copy link
Contributor Author

@swift-ci please smoke test

Copy link
Member

@lorentey lorentey left a comment

Choose a reason for hiding this comment

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

This looks good to me. I had two comments, but given the existing benchmarks, it may be best to ignore them. (Local consistency may be more valuable than global in this case.)

public func run_UTF8Decode_InitFromData_ascii_as_ascii(_ N: Int) {
let input = asciiData
for _ in 0..<1_000*N {
blackHole(String(data: input, encoding: .ascii))
Copy link
Member

Choose a reason for hiding this comment

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

input stays constant over the duration of the for statement, so I wonder if a sufficiently clever optimizer would be able to move some of its processing outside the loop.

It may be worth passing input through identity. (Then again, the other benchmarks don't do this, and we should stay consistent if we want to be able to compare results,)

@@ -42,6 +42,18 @@ public let UTF8Decode = [
name: "UTF8Decode_InitFromBytes_ascii",
runFunction: run_UTF8Decode_InitFromBytes_ascii,
tags: [.validation, .api, .String]),
BenchmarkInfo(
name: "UTF8Decode_InitFromData_ascii_as_ascii",
Copy link
Member

Choose a reason for hiding this comment

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

Hm, the names aren't great here. Shouldn't we call this ASCIIDecode.initFromData?

The existing benchmarks have set an unfortunate pattern, but perhaps it's worth breaking it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I felt keeping the prefix the same was worthwhile for being able to do something like "run all the benchmarks for X". The name does really suck though.

@Catfish-Man Catfish-Man merged commit d7d903f into swiftlang:master Oct 14, 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