Skip to content
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

Performance improvements #1121

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fmichaut-diff
Copy link

Opening this pull request cause we noticed that this gem was making duplicate queries for the same resources on our app, and decided to dig in to find the root cause.

The main usecases were :

  • On delete, we notice that we the gem delete a Tagging, and then query the same Tag twice
  • On save! with nothing changed, the gem queries existing Tags and Tags in current context, even tho nothing changed so the gem should not have to query anything
  • On save! with changes (or not), the gem seems to query the same contexts multiple times, which is redundant

Here are improvements for the first and last usecase. The middle one seem more complex, and is left for a future improvement.

Improvements

For the delete improvement :
We simply only reload the tag record if already loaded. Otherwise tag.reload would load it twice.

For the save! improvement :
Since we initialize custom_contexts with taggings.map(&:context).uniq, but tagging_contexts uses both self.class.tag_types.map(&:to_s) and custom_contexts, we have duplicates contexts in tagging_contexts which results in the gem attempting to save each context twice.
Add a uniq to tagging_contexts to ensure that does not happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant