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

Large search indices block UI on page load (initSearch) #1210

Open
jonasViehweger opened this issue Mar 29, 2023 · 4 comments
Open

Large search indices block UI on page load (initSearch) #1210

jonasViehweger opened this issue Mar 29, 2023 · 4 comments
Labels
bug status: needs discussion Issues that need more discussion before they can be properly triaged.

Comments

@jonasViehweger
Copy link

Describe the bug
For larger search indices (in this case 2.7 MB) the search initialization blocks the UI for a few seconds on new page load.

To Reproduce
Steps to reproduce the behavior:

  1. Fork this repo
  2. Set search to true in _config.yml
  3. Build the site locally
  4. Navigate and observe the UI locking up at page load

Or you can use this search-data file to build a test page:

search-data.zip

Expected behavior
Being able to navigate and interact with the site quickly after load should have priority over loading the search. I actually tried a few different things to get that to work, like RequestIdleCallback to build the index with lunr, or pre-building the search index (see #378). Which for both I couldn't really make work without locking up the page somehow.

One thing that would work quite well is only loading the search index when focusing on the search bar. Something like what is proposed in the docsy repo here: google/docsy#1024

This would have the benefit of only locking up the site when the search is loaded, and you could have a loading indicator. For small indices nothing should really change from the handling since the search index is built almost immediately anyway.

I could do a PR for that myself, but I am absolutely useless in javascript so the code I would submit might work but would likely be bad style. But I could give it a try.

Desktop (please complete the following information):

  • OS: Ubuntu 20.04
  • Firefox 111
@mattxwang
Copy link
Member

mattxwang commented Mar 29, 2023

Thanks for the detailed issue report @jonasViehweger, and links to relevant related projects.

I agree that loading search indices shouldn't be render-blocking. Here's the workflow I'm envisioning:

  1. on page load:
    • plop a spinner / "loading..." into the search bar
    • async/Promisfy every step of the pipeline:
      • get the json file (pretty sure this is already async)
      • parse the JSON
      • looping through the docs (this may be unnecessary)
      • lunr function stuff (this may be challenging)
  2. on successful promise
    • remove the spinner, load the index; i.e. the current behaviour
  3. on failed promise
    • display an error message

I will say, I'm surprised that RequestIdleCallback didn't work. I know lunr hasn't been updated in a while, but I don't think any major web promise/async APIs have changed in the past ~ 5 years.

I'll sleep on this and think a bit more about alternative solutions. In the meantime, if you'd like to submit a PR, that's also welcome! I'm happy to work with you to make sure it's in good shape. That being said, feel free to not take it up; I can always add it to my backlog.

Separately - I think it'd be good to get a more rigorous understanding (from a profiling perspective) of exactly where this bottleneck happens. What is the proportion of:

  • render-blocking JSON loading (this should be async already)
  • parsing the actual JSON file (this seems to be synchronous, and could become a promise)
  • looping through the generated JSON
  • time spent in the lunr function itself (this would require us to change some lunr source code

In contrast, I'm not a fan of loading the index only when the user clicks the search box, since this punts the problem down the road on the user's end. Starting the async job as soon as the user loads the page (but not making it render-blocking) seems like the best step forward.

Open to other suggestions; I'm not particularly wedded to this idea!

@mattxwang
Copy link
Member

Oh, and on an unrelated note - given the size of your site, I would suggest trying to upgrade Ruby and Jekyll versions; Ruby's 3x3 speedup for Ruby 3 is quite nice, and it seems like Jekyll 4 has good performance gains for large sites. Something I noticed while testing your repo locally -- it takes a while to generate 😅

@jonasViehweger
Copy link
Author

jonasViehweger commented Mar 29, 2023

Thanks for the quick reply (and for the suggestion about build time). I agree that moving the init to focus is not an ideal solution but in my opinion it is a quick solution which is better in every way than the current situation. It will not negatively affect sites with small indices, and since searching is likely a rarer operation than navigating it will give big benefits to sites with larger search-data since the blocking will only happen the few times the user decides to search. How it is right now blocking will happen every time, regardless if the search is used or not.

But I do agree that a non blocking function is of course ideal.

From benchmarking with firefox it looks like the lunr insert function, specifically minimizing, is taking most of the time (~1s) with JSON parsing next in line taking around a quarter of a second.

Full Disclaimer about RequestIdleCallback: I have no clue what I'm doing, I was just roughly following what was done in this blog. So it would probably bring benefits, just not the way I had it implemented.

@mattxwang mattxwang added the status: needs discussion Issues that need more discussion before they can be properly triaged. label Apr 8, 2023
@mattxwang
Copy link
Member

Gotcha, thanks! Hm, I think we should do it right the first time, and would prefer not incurring a cost on opening the search bar for small sites. However, I'd welcome a PR for either approach - @jonasViehweger, let me know if you'd like to implement it!

(if not, it goes into my backlog, but that's getting quite big 😅 )

@mattxwang mattxwang changed the title initSearch blocks UI with large search-data.json Large search indices block UI on page load Apr 8, 2023
@mattxwang mattxwang changed the title Large search indices block UI on page load Large search indices block UI on page load (initSearch) Apr 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug status: needs discussion Issues that need more discussion before they can be properly triaged.
Projects
None yet
Development

No branches or pull requests

2 participants