-
-
Notifications
You must be signed in to change notification settings - Fork 400
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
add benchmarks for get_for_dialect
#1862
add benchmarks for get_for_dialect
#1862
Conversation
these are high-level-benchmarks that will somehow call Field.get_for_benchmark
CodSpeed Performance ReportMerging #1862 will not alter performanceComparing Summary
Benchmarks breakdown
|
Pull Request Test Coverage Report for Build 13008190876Details
💛 - Coveralls |
tests/benchmarks/test_annotate.py
Outdated
loop.run_until_complete(_bench()) | ||
|
||
|
||
def test_values_related_m2m(benchmark, create_team_with_participants): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one doesn't have .annotate
but it is located in the test_annotate
. Maybe, move to test_filter.py
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved it to test_relations.py
for now. Maybe we can add more tests benchmarking relational behaviour in the future
tests/benchmarks/test_annotate.py
Outdated
from tortoise.functions import Count | ||
|
||
|
||
def test_function_count(benchmark, few_fields_benchmark_dataset): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the functions in this file are going to be the names of the benchmarks, so it would be nice to name them well:
test_annotate_with_count
test_annotate_with_decimal
- etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_annotate.py
was a working title I did not change yet, sorry for that. I simply wanted some functions that call get_for_dialect
, so I went ahead and looked where the function is used, placed a 1/0
before/after the function call, ran make ci
and hoped that an error is thrown somewhere. Then, I copied these "failing" tests and adopted them as a benchmark. Duriung this, I skipped everything in schema_generator.py
because this is only executed once during startup and would not benchmark any real-world scenario.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
these are high-level-benchmarks that will somehow call
Field.get_for_dialect
Description
The above mentioned method has some potential to be optimized.
Motivation and Context
I want to optimize the method, like in this comment: #1859 (comment)
It is not used very often, but for example if you call any tortoise.expressions.Function, filter through/retrieve values of related models,...
How Has This Been Tested?
there seams to be a slight improvement with my optimizations
Checklist: