refactor: benchmarks tidied up #308
Conversation
|
@Alhanaqtah hey! Sorry for a quite late answer. Could you please fix issues I've left and also do following:
|
| shell: bash | ||
| run: | | ||
| COUNT=$([[ "${{ github.event_name }}" == "pull_request" ]] && echo 1 || echo 5) | ||
| COUNT=$([[ "${{ github.event_name }}" == "pull_request" ]] && echo 5 || echo 5) |
There was a problem hiding this comment.
One of several goals of this issue is reducing benchmarks execution time therefore increase amount of benchmarks iterations.
Could you increase COUNT:
- for pull-requests to 10;
- otherwise to 15;
There was a problem hiding this comment.
I face this error becaouse previously fixed continuous-banchmarks.yml:
[remote rejected] 263-tidy-up-benchmarks -> 263-tidy-up-benchmarks (refusing to allow a Personal Access Token to create or update workflow `.github/workflows/continuous-benchmarks.yml` without `workflow` scope)
So you'll have to do it)
There was a problem hiding this comment.
Try to rebase this branch onto main first. I guess it will fix the issue.
| } | ||
|
|
||
| func BenchmarkBitmask(b *testing.B) { | ||
| func BenchmarkBitmask_HasBitsIn(b *testing.B) { |
There was a problem hiding this comment.
Let's stick with previous naming to preserve history of measurements.
FYI, we track them here - https://ozontech.github.io/seq-db/
There was a problem hiding this comment.
Wow, that's interesting. Do you track the bench results in each release to find regressions?
There was a problem hiding this comment.
Almost. For each commit that gets merged into main branch.
| } | ||
| } | ||
|
|
||
| func BenchmarkGenerateDocsJSON(b *testing.B) { |
There was a problem hiding this comment.
IMHO, these benchmarks are useless. They evaluate performance of functions which are used only in test (and only in integration tests, AFAIK).
What do you think about removing such benchmarks?
There was a problem hiding this comment.
I agree with you. Maybe I should also delete benchmarks for functions that are not in use: GenerateDocsJSON, GenerateDocs (maybe delete the functions themselves)?
| } | ||
| wg.Wait() | ||
| logger.Info("list", zap.Int("total", len(list))) | ||
| b.Logf("list_total=%d", len(list)) |
There was a problem hiding this comment.
It's better to remove any method invocations that can affect measurements and which are not part of the benchmarkable code.
|
|
||
| // bench for base point | ||
| // you can't go faster | ||
| func BenchmarkCopy(b *testing.B) { |
There was a problem hiding this comment.
Let's skip this benchmark as well.
| } | ||
|
|
||
| // base point | ||
| func BenchmarkIterate(b *testing.B) { |
There was a problem hiding this comment.
Let's skip this benchmark as well.
| } | ||
| } | ||
|
|
||
| func BenchmarkStatic(b *testing.B) { |
There was a problem hiding this comment.
Let's skip this benchmark as well.
2 participants
Description
The tests have been finalized to use the new API (
b.Loop()). Also, the dependence onb.Nin determining the size of the generated data has been eliminated.If you have used LLM/AI assistance please provide model name and full prompt: