perf: Optimize db queries for preview panel #942
Conversation
TheBobBobs
force-pushed
the
perf/preview_panel
branch
from
4ef7c13 to
59460e4
Compare
CyanVoxel
added
TagStudio: Search
CyanVoxel
left a comment
CyanVoxel
left a comment
There was a problem hiding this comment.
I apologize for getting around to this one so late. Everything seems to be working as intended and I'm glad that the tag hierarchy logic is now finally back into its own method. Thank you for working on this, and great changes!
CyanVoxel
left a comment
CyanVoxel
left a comment
There was a problem hiding this comment.
Sorry to backtrack on this, but it appears that this conflicts with a recent bugfix we had to correct an issue where the parent_id and child_id columns were erroneously flipped in the tag_parents table (#998). I attempted to update the logic in get_tag_hierarchy() and get_tag_categories(), but I'm not as familiar with these changes compared to the old code and wasn't able to get it working on my own. This also illuminated some quirks in the logic of get_tag_hierarchy() that resulted in the issue propagating and becoming more difficult to track down.
Would you mind seeing if you can update this to the latest code on main?
| for tag in tags: | ||
| all_tags[tag.id] = tag | ||
| for tag in all_tags.values(): | ||
| tag.parent_tags = {all_tags[p] for p in all_tag_parents.get(tag.id, [])} |
There was a problem hiding this comment.
Returning Tag objects with overwritten parent data resulted in the conflict issue propagating further in the UI stack, including flipping the tag's parents with its children in the BuildTagPanel due to the UI sharing that same "tainted" Tag object. While this shouldn't be an issue when the logic if flipped to match main, it still presents the question of whether or not this may mask further issues in the future.
There was a problem hiding this comment.
Not manually setting fields on db objects would definitely be better.
Later this week I'll try experimenting with using joins for this data to see if it can be done without a performance difference.
After some testing I also found out this affects the state tracking sqlalchemy does. Which was causing issues when trying to save tag changes when the edit ui was opened from the preview panel.
The solution is pretty simple but I couldn't find any documentation for the fix I did.
There was a problem hiding this comment.
Sounds good! I'll go ahead and merge this PR in to main to get the rest of these changes live
github-project-automation
bot
moved this from 👀 In review
to ✅ Done
in TagStudio Development
2 participants
Summary
Add method
get_tag_hierarchytoLibrarythat returns adict[TagId, list[ParentId]]anddict[TagId, Tag]containing every tag in the hierarchy. UpdateFieldContainersto use this new method.Currently all places that call
Library.tag_display_namealready have theTagobject so we don't need to fetch it from db.Tasks Completed