ci(bench): migrate benchmark tool benny to vitest #297
Conversation
Signed-off-by: sunrabbit123 <[email protected]>
|
Interesting, so because of the ordering of test cases, the current benchmarks where biased toward native implementation and vitest's results show that ts-pattern's performance are more similar to native control flow statements. Am I interpreting this correctly? |
benchmarks/nested-objects.bench.ts
Outdated
| const input = (() => { | ||
| const map = { | ||
| 0: { type: 'a' as const, value: { x: Math.random(), y: Math.random() } }, | ||
| 1: { | ||
| type: 'b' as const, | ||
| value: Math.random() > 0.5 ? [1, 2, 3, 4] : ['hello'], | ||
| }, | ||
| 2: { type: 'c' as const, age: Math.random(), name: 'acdfl' }, | ||
| }; | ||
|
|
||
| return map[inputIndex]; | ||
| })(); |
There was a problem hiding this comment.
We should re-generate the input on every run instead of only once, otherwise we are always testing against the same object which could make results vary between runs, and could result in irrelevant JIT optimizations.
There was a problem hiding this comment.
Nevertheless, the object creation time should be excluded from the benchmark.
Therefore, I am looking for ways to use setup.
I will ask you to review it again after working on it.
There was a problem hiding this comment.
I don't mind that the object creation is included in the benchmark as long as it's included in all cases. Its only purpose is to compare equivalent implementation of the same thing, so the absolute values aren't that important
benchmarks/random-digit.bench.ts
Outdated
| const rand = () => Math.floor(Math.random() * 10) as Digit; | ||
|
|
||
| describe('ts-pattern-benchmark/random-digit', () => { | ||
| const digit = rand(); |
Signed-off-by: sunrabbit123 <[email protected]>
What is certain is that Benny is a problematic benchmark tool. As for whether the beatest is correct, I think it is better to check the benchmark result one more time after modifying it according to the review. |
Signed-off-by: sunrabbit123 <[email protected]>
|
next is changed benchmark result
I still don't feel the big gap. |
The reason why I raised the PR is as follows.
Proposal
If this PR is accepted, there is also a foothold to replace all tests with bitest-based rather than just test.