[benchmark] Fix the Dictionary(group:by:) test #10306
Conversation
|
@swift-ci Please smoke test and merge |
|
@swift-ci Please smoke test OS X platform |
|
@swift-ci Please test |
|
Build failed |
|
We had more breakage on master. Should be fixed now... Let's try this again. |
|
Build failed |
|
@swift-ci Please test |
|
Build failed |
|
@swift-ci Please test |
| @@ -39,9 +41,11 @@ class Box<T : Hashable> : Hashable { | |||
|
|
|||
| @inline(never) | |||
| public func run_DictionaryGroupOfObjects(_ N: Int) { | |||
| let count = 20 * N | |||
| for _ in 1...N { | |||
| let count = 5_000 | |||
| let result = count / 10 | |||
| let dict = Dictionary(grouping: (0..<count).map { Box($0) }, by: { Box($0.value % 10) }) | |||
There was a problem hiding this comment.
@natecook1000 This map is eager. Wouldn’t it make sense to create the [Box] outside of the top level for loop? I’m guessing your goal isn’t to measure the Box object creation, but the performance of grouping on reference counted objects on the heap, right?
There was a problem hiding this comment.
How about this?
let count = 5_000
let result = count / 10
let objects = (0..<count).map { Box($0) }
for _ in 1...N {
let dict = Dictionary(grouping: objects, by: { Box($0.value % 10) })
CheckResults(dict.count == 10)
CheckResults(dict[Box(0)]!.count == result)
}There was a problem hiding this comment.
Also, as the N get's balanced by the benchmark driver, why not make the count same between DictionaryGroup and DictionaryGroupOfObjects, so that their respective ratio better reflects the change in work going from structs to classes?
There was a problem hiding this comment.
@dabrahams What do you think?
There was a problem hiding this comment.
@palimondo All good points, thanks!
When testing quadratic behavior, remember to repeat small amounts of the behavior several times, not to increase the N, which is squared. 🙄
natecook1000
force-pushed
the
nc-grouping-test
branch
from
ac36c0e to
ced26b8
Compare
|
@swift-ci Please test |
|
Build failed |
|
Build failed |
|
@natecook1000 Did you mean "please benchmark"? |
|
No, the change to benchmark is in #10275 |
|
@swift-ci Please smoke test OS X platform |
When testing quadratic behavior, remember to repeat small amounts of the behavior several times, not to increase the N, which is squared. 🙄