Skip to content

feat: improve performance for GET /api/album & /api/album/:id #17124

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

Merged
merged 5 commits into from
Mar 31, 2025

Conversation

PathToLife
Copy link
Contributor

@PathToLife PathToLife commented Mar 26, 2025

Description

Performance improvement for GET /api/albums when there are >10K assets in multiple albums

This is a significant UX improvment for first load page performance and repeated navigation into and out of albums (depending on browser caching)

Query Times

Average BEFORE 1.78s
Average AFTER 0.67s
Difference -1.11s

Improvement 62.3%

From:
image

To:
image
Fixes # (issue)

How Has This Been Tested?

Find a server with albums at least 2-3 albums with over 30k assets in each

Remember to disable browser cache just in case
image

  • Load the albums page, go into album page, come back. There should be a noticable reduction in lag

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if applicable
  • I have no unrelated changes in the PR.
  • I have confirmed that any new dependencies are strictly necessary.
  • I have written tests for new code (if applicable)
  • I have followed naming conventions/patterns in the surrounding code
  • All code in src/services/ uses repositories implementations for database calls, filesystem operations, etc.
  • All code in src/repositories/ is pretty basic/simple and does not have any immich specific logic (that belongs in src/services/)

Thanks to @mertalev for helping with more indepth analysis of SQL queries! :)

@PathToLife PathToLife marked this pull request as ready for review March 26, 2025 03:22
remove unnecessary join for getMetadataForIds
remove separate call to getLastUpdatedAssetForAlbumId
… GET /api/album/:id

also remove getLastUpdatedAssetForAlbumId query as it is no longer referenced
@PathToLife PathToLife force-pushed the fix/album-list-performance branch from 9e01573 to 966c3f2 Compare March 26, 2025 03:22
@alextran1502 alextran1502 requested a review from mertalev March 28, 2025 15:02
tests & lint still pass before this commit.
@zackpollard zackpollard changed the title fix(server) performance GET /api/album, GET /api/album/:id feat: improve performance for GET /api/album & /api/album/:id Mar 31, 2025
@zackpollard zackpollard enabled auto-merge (squash) March 31, 2025 11:10
@zackpollard zackpollard merged commit 09f4476 into immich-app:main Mar 31, 2025
43 of 44 checks passed
kirill-dev-pro pushed a commit to kirill-dev-pro/immich-but-with-public-albums-within-instance that referenced this pull request Apr 6, 2025
…-app#17124)

* fix(server) optimize number of sql calls for GET /api/albums

remove unnecessary join for getMetadataForIds
remove separate call to getLastUpdatedAssetForAlbumId

* fix(server) remove unnecessary getLastUpdatedAssetForAlbumId call for GET /api/album/:id

also remove getLastUpdatedAssetForAlbumId query as it is no longer referenced

* fix(server): correct lastModifiedAssetTimestamp return type + formatting and typing

* chore(server): address type issue with tests found via npm:check

tests & lint still pass before this commit.
savely-krasovsky pushed a commit to savely-krasovsky/immich that referenced this pull request Jun 8, 2025
…-app#17124)

* fix(server) optimize number of sql calls for GET /api/albums

remove unnecessary join for getMetadataForIds
remove separate call to getLastUpdatedAssetForAlbumId

* fix(server) remove unnecessary getLastUpdatedAssetForAlbumId call for GET /api/album/:id

also remove getLastUpdatedAssetForAlbumId query as it is no longer referenced

* fix(server): correct lastModifiedAssetTimestamp return type + formatting and typing

* chore(server): address type issue with tests found via npm:check

tests & lint still pass before this commit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants