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

[Perf] Move away from the APIs that return all tags in one request #528

Closed
MohamedBassem opened this issue Oct 13, 2024 · 12 comments
Closed

Comments

@MohamedBassem
Copy link
Collaborator

People with thousands of tags are already feeling slowness in viewing their tags or searching through them. This issue is to:

  1. Change the AllTags page to be paginated (with a search at top).
  2. Change the tag editor to do async search requests to the backend instead of inline all the tags.
@Arcturuss
Copy link

I feel there is something wrong with the page layout, if just a thousands (not even millions) of tags is enough to slow the browser. Same goes for the API request. Maybe it will be better in the long run to fix the underlying cause instead of hiding it with pagination?

@petrm
Copy link

petrm commented Nov 5, 2024

I just started to notice a huge slowdown with 5000 or so tags. Looking at the performance of the page, the browser actually spends most of the time waiting for /dashboard/tags, in my case over 9 seconds.
It doesn't look to me like a problem in the browser, but in the backend. Is there any flag that could be enabled to spit out the SQL query used to generate the tags on the page? 5000 rows is nothing - this has to return results in milliseconds.

@petrm
Copy link

petrm commented Nov 5, 2024

Well, I went to the drizzle docs, looks like there would be a way to show the query, but as far as I can see it would need to be explicitely added to the hoarder codebase https://orm.drizzle.team/docs/goodies#printing-sql-query. Also there is drizzle-team/drizzle-orm#2605.

@MohamedBassem
Copy link
Collaborator Author

it's unlikely that the problem is in the query itself. Last time we've seen this problem was because of the rendering (and potentially the server side rendering in this case).

@petrm
Copy link

petrm commented Nov 5, 2024

It can be that previously it was, but look at this:
image
Would you be able to share the queries?

@MohamedBassem
Copy link
Collaborator Author

Most likely that's server side rendering taking the time. I can share the queries when I'm in front of the laptop.

@kamtschatka
Copy link
Contributor

{
  sql: 'select "id", "name", "createdAt", "userId", (select coalesce(json_group_array(json_array("attachedBy")), json_array()) as "data" from "tagsOnBookmarks" "bookmarkTags_tagsOnBookmarks" where "bookmarkTags_tagsOnBookmarks"."tagId" = "bookmarkTags"."id") as "tagsOnBookmarks" from "bookmarkTags" where "bookmarkTags"."userId" = ?',
  params: [ 't9uxin6b6q7fz1v72jm31nr6' ],
  typings: [ 'none' ]
}

@petrm
Copy link

petrm commented Nov 5, 2024

Thanks. I have been working with the assumption that the server just spits out json which is then used by the browser. I guess I will have to set up a dev copy and dig into typescript profiling ;)

@kamtschatka
Copy link
Contributor

so yes, the query does take surprisingly long (1,5 seconds with 5000 tags). Then we calculate the ai/human part, which takes a few ms.
The whole request takes 3,5 seconds on my machine, so 2 seconds server side rendering.

kamtschatka added a commit to kamtschatka/hoarder-app that referenced this issue Nov 5, 2024
…arder-app#528

improved the query speed by getting rid of the "with" part of the query and doing that manually
@kamtschatka
Copy link
Contributor

I changed the way the query is done and it reduces the time to below 100ms on my machine.
Rendering and transfering the data from the server will still take 2 seconds or so.
I guess we should start testing with much larger bookmark/tag amounts to find similar bottlenecks and see what we can do.

@github-project-automation github-project-automation bot moved this from Backlog to Done in Hoarder's Roadmap Nov 8, 2024
@MohamedBassem
Copy link
Collaborator Author

@kamtschatka Can you try now after the index fix?

@kamtschatka
Copy link
Contributor

aaahh, i have checked the index, but was too blind to see the misconfiguration...
I have tried it and the query is now just as fast as with my solution (always sub 100ms, mostly closer to 50ms)
I am wondering though if client side rendering of the tags page would be still faster, since creating the HTML and transferring it to the client also contributes a lot to the load times.
A NAS is usually less powerful than a PC and the upload also takes some time.
Unfortunately it was not as easy as adding "use client;" to the files, so I was not able to try that....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

5 participants
@petrm @Arcturuss @MohamedBassem @kamtschatka and others