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

Add in-memory user cache #3599

Merged
merged 1 commit into from
Nov 10, 2024
Merged

Add in-memory user cache #3599

merged 1 commit into from
Nov 10, 2024

Conversation

mikiher
Copy link
Contributor

@mikiher mikiher commented Nov 10, 2024

This PR Addresses point 5 from the conclusions of the image loading performance analysis.

An in-memory cache check and insert is introduced in all user model methods that return a single user (with their media progress) from the database. Caching this saves around 15 ms per authenticated request.

The cache doesn't have a TTL, and is invalidated only if needed (i.e. when changes to the user were not made through the cached object). I have convinced myself that all relevant writes to user and media progress are made through user model methods that are covered by a call to the maybeInvalidate method (and specifically in createUpdateMediaProgressFromPayload)

Note that this covers point 5, but I've decided not address point 4 (Remove media progress from req.user), since at this point media progress is quite entangled with req.user and requires quite a few changes in the code, which aren't worth it since the cache already solves the db performance issue. I think it would still be worthwhile disentangle them in the future, but this change makes it less urgent.

@mikiher mikiher marked this pull request as ready for review November 10, 2024 07:01
@advplyr
Copy link
Owner

advplyr commented Nov 10, 2024

Thanks!

@advplyr advplyr merged commit 01446c0 into advplyr:master Nov 10, 2024
5 checks passed
@mikiher mikiher deleted the add-user-cache branch November 18, 2024 07:02
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.

2 participants