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

Delay store.UserByID as much as possible #2986

Merged
merged 2 commits into from
Dec 10, 2024
Merged

Conversation

jvoisin
Copy link
Collaborator

@jvoisin jvoisin commented Dec 8, 2024

In internal/reader/handler/handler.go:RefreshFeed, there is a call to store.UserByID pretty early, which is only used for originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language) Its only other usage is in processor.ProcessFeedEntries(store, originalFeed, user, forceRefresh), which is pretty late in RefreshFeed, and only called if there are new items in the feed. It makes sense to only fetch the user's language if the error localization function is used.

This commit also makes processor.ProcessFeedEntries take a userID instead of a user, to make the code a bit more concise.

This should close #2984

In internal/reader/handler/handler.go:RefreshFeed, there is a call to
store.UserByID pretty early, which is only used for
originalFeed.WithTranslatedErrorMessage(localizedError.Translate(user.Language)
Its only other usage is in processor.ProcessFeedEntries(store, originalFeed,
user, forceRefresh), which is pretty late in RefreshFeed, and only called if
there are new items in the feed. It makes sense to only fetch the user's
language if the error localization function is used.

Calls to `store.UserByID` take around 10% of the CPU time of RefreshFeed in my
profiling.

This commit also makes `processor.ProcessFeedEntries` take a `userID` instead
of a `user`, to make the code a bit more concise.

This should close miniflux#2984
@fguillot fguillot merged commit 637fb85 into miniflux:main Dec 10, 2024
16 checks passed
@jvoisin jvoisin deleted the delay_sql branch December 26, 2024 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Delay store.UserByID in RefreshFeed
2 participants