Significant performance improvement across tagGalleriesFromImages#721
Significant performance improvement across tagGalleriesFromImages#721DogmaDragon wants to merge 3 commits into
Conversation
|
This pull request has been mentioned on Stash Forum. There might be relevant details there: https://discourse.stashapp.cc/t/tag-galleries-from-images/3904/3 |
spaceyuck
left a comment
spaceyuck
left a comment
There was a problem hiding this comment.
This seems like a lot of code change noise for a few smallish actual changes:
- paged gallery loading (fair enough, didn't expect people to have that many galleries)
- studio gets pushed to gallery too
- hook into image changes (incomplete)
| if exclussion_marker_tag is not None: | ||
| exclusion_marker_tag_id = exclussion_marker_tag['id'] | ||
| if settings["excludeWithTag"]: | ||
| if settings["excludeWithTag"] in tag_cache: |
There was a problem hiding this comment.
I really don't get the point of the tag_cache. processAll() gets invoked once, so there are no repeat lookups of the same thing to cache. And even then, it's a single tag lookup by name at worst. Should be removed if pointless, just added complexity.
| processed = 0 | ||
| page = 1 | ||
|
|
||
| while True: |
There was a problem hiding this comment.
Not a huge fan of while loops without an explicit abort condition if there's an option to have one. Any reason processed < total_count is not feasible anymore?
| missing_perfs = all_found_performer_ids - existing_perf_ids | ||
|
|
||
| target_studio_id = list(all_found_studio_ids)[0] if all_found_studio_ids else None | ||
| studio_missing = bool(target_studio_id and (target_studio_id != existing_studio_id)) |
There was a problem hiding this comment.
Performers and tags are additive changes (so manually added tags and performers remain unchanged), but the studio gets overwritten if already set (manually or otherwise). This seems worth talking about, and maybe should be a setting to be enabled if wanted behavior by user.
|
|
||
| update_data = {"ids": [gallery_id]} | ||
| if missing_tags and all_found_tag_ids: | ||
| update_data["tag_ids"] = {"mode": "ADD", "ids": list(all_found_tag_ids)} |
There was a problem hiding this comment.
Why are all tags and performers added again, if the missing IDs are already in missing_tags and missing_tags? Seems like pointless extra work for the server.
| processGallery(gallery) | ||
|
|
||
| # --- IMAGE HOOK HANDLER --- | ||
| elif hook_type in ["Image.Update.Post", "Image.Create.Post"]: |
There was a problem hiding this comment.
These image hooks are not declared in the manifest, and will never be called.
Also, should these be active by default? Every hook invocation already makes bulk update operations slower, and the images are not the ones getting changed, so I'd probably add a setting that has to be enabled to be more than a NO-OP
4 participants
Related PR #718