[Benchmarks] Add a whole lot more benchmarks for Data #20396
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@swift-ci please smoke test |
|
@swift-ci please smoke benchmark |
phausler
commented
Nov 7, 2018
| blackHole(Data([0, 1, 2, 3, 4, 5, 6, 0, 1, 2, 3, 4, 5, 6, 0, 1, 2, 3, 4, 5, 6, 0, 1, 2, 3, 4, 5, 6])) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
@milseman What other converters do we want to test here? Should we test the Foundation overlay variations as well?
There was a problem hiding this comment.
Up to you. Next release, we'll want to unify them as part of some upcoming string spring cleaning.
itaiferber
approved these changes
Nov 7, 2018
Build comment file:Performance: -O
Code size: -O
Performance: -Osize
Code size: -Osize
Performance: -Onone
How to read the dataThe 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 |
|
Among other things, this has also reintroduced setup overhead to some of the benchmarks I previously cleaned up. All the new Also all the I'll clean this up, but in the future, please do run Benchmark Check Report
|
|
Large variants are useful in the regards that it takes a completely different code path and has different performance characteristics. I understand we should not do that for every test but it still should not regress imho. The arc4random is being used to ensure the kernel does not just hand out a non faulting page of memory. Just being an allocation is not enough to properly identify used cases. It must be written to: and patterns like repeated are a completely different code path in the sequence initialization. |
|
If we want to transition to a different random source that is fine in my book. It does not need to be arc4 but it should be something portable to Linux is all. |
|
Can you clarify this part?
To the best of my understanding, even the func sampleData(size: Int) -> Data {
var data = Data(count: size)
data.withUnsafeMutableBytes { getRandomBuf(baseAddress: $0, count: size) }
return data
}How is it different initialization pattern than this from IterateData? var data = Data(count: 16 * 1024)
let n = data.count
data.withUnsafeMutableBytes { (ptr: UnsafeMutablePointer<UInt8>) -> () in
for i in 0..<n {
ptr[i] = UInt8(i % 23)
}
}I think that it is fully equivalent from the perspective of |
|
Oh that differential is fine, I was just saying that Repeated (the sequence type) would behave differently. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a number of new benchmarks for Data that test specific common cases of empty, small, medium and large Datas. Additionally this adds new benchmarks for creation as well as conversion from String to Data using Sequence/Collection initializers.