[WIP] Optimized jobs page and IAM information retrival #8156
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe updates enhance the efficiency and accuracy of data retrieval in serializers and views, streamline query performance by leveraging Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://github.com/avatars/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- cvat/apps/engine/serializers.py (1 hunks)
- cvat/apps/engine/views.py (4 hunks)
- cvat/apps/iam/permissions.py (2 hunks)
Additional comments not posted (5)
cvat/apps/iam/permissions.py (1)
42-42: Simplification ofget_organizationfunction is effective but needs verification.The new implementation is more straightforward and reduces potential error points. However, ensure that the simplification does not overlook any edge cases that the previous logic handled.
Verification successful
Simplification of
get_organizationfunction is verified and does not introduce any errors.The new implementation is straightforward and effectively handles the necessary cases without overlooking any edge cases.
- The
get_organizationfunction correctly returns theorganizationattribute orNone.- The usage of
get_organizationinget_iam_contextis appropriate and functions as expected.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `get_organization` function to ensure the new implementation does not introduce any errors. # Test: Search for the function usage. Expect: No breaking changes due to simplification. rg --type python -A 5 $'get_organization'Length of output: 71
Script:
#!/bin/bash # Description: Verify the usage of `get_organization` function to ensure the new implementation does not introduce any errors. # Test: Search for the function usage. Expect: No breaking changes due to simplification. rg -A 5 'get_organization'Length of output: 2897
Script:
#!/bin/bash # Description: Verify the detailed context of `get_organization` function usage in `cvat/apps/iam/permissions.py`. # Test: Search for the function usage with more surrounding lines for context. rg 'get_organization' -A 10 -B 10 cvat/apps/iam/permissions.pyLength of output: 1426
cvat/apps/engine/serializers.py (1)
1347-1352: Optimize generator expressions and handle empty cases gracefully.The generator expressions for
subsetsanddimensionsimprove efficiency by avoiding the creation of intermediate lists. However, there are a few points to consider:
- The generator expressions are executed twice, which might result in a performance hit. Consider storing the results in variables if the task list is large.
- The use of
nextfordimensionsis appropriate for handling empty cases gracefully.- Ensure the
task_subsetsset handles empty strings correctly.+ subsets = (task.subset for task in instance.tasks.all()) + dimensions = (task.dimension for task in instance.tasks.all()) + task_subsets = set(subsets) + task_subsets.discard('') + response['task_subsets'] = list(task_subsets) + response['dimension'] = next(dimensions, None)cvat/apps/engine/views.py (3)
292-293: LGTM! The conditional block optimizes query performance.The added condition in the
get_querysetmethod for thepreviewaction filters the queryset based on thekwargs, which should enhance performance for this specific action.
630-630: LGTM! The change ensures the most recent task is used for the preview.The
previewmethod now selects the first task related to the project usingselect_relatedfor thedatafield and orders by-id, ensuring that the most recent task is used for the preview image.
869-870: LGTM! The conditional block optimizes query performance.The added condition in the
get_querysetmethod for thepreviewaction filters the queryset based on thekwargsand usesselect_relatedfor thedatafield to optimize query performance.
bsekachev
changed the title
bsekachev
changed the title
bsekachev
force-pushed
the
bs/queries_optimization_2
branch
from
1c699e8
to
8de3127
Compare
bsekachev
force-pushed
the
bs/queries_optimization_2
branch
from
bd1fb4f
to
6a71628
Compare
|
|
Will start from scratch |
Motivation and context
In a task with 19 jobs (actually it worked O(number of jobs))
Before:
After: ( still O(number of jobs), but already better):
How has this been tested?
Checklist
developbranch(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
Performance Improvements
Bug Fixes
Backend Enhancements
ProjectViewSet,TaskViewSet, andJobViewSetwith aselect_relatedclause for better performance and reduced database hits.Code Simplification
get_organizationfunction, making it more straightforward and removing unnecessary exception handling.