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

searchtools: Don't use slideDown to show search results. #8868

Merged
merged 1 commit into from Feb 11, 2021

Conversation

emilio
Copy link
Contributor

@emilio emilio commented Feb 10, 2021

Subject: Don't use slideDown to show search results.

Feature or Bugfix

  • Bugfix

Purpose

Detail

Fixes a performance issue with massive lists like #8562.

jQuery.slideDown is pretty bad perf-wise since it thrashes layout
multiple times on all browsers. This is specially bad on slow machines.

The 5ms animation is unnoticeable (it last less than an animation
frame!) and just burns CPU cycles unnecessarily in all browsers.

On Firefox this is specially bad because it hits a performance cliff
with list items / CSS counters
(https://bugzilla.mozilla.org/show_bug.cgi?id=1683910#c26 for all the
gory details) where it causes tons of counter recalc.

An alternative fix for the Firefox cliff would be to set overflow:
hidden on the list item, but I think removing the animation is probably
more sensible, given that as I said it's just burning CPU.

Fixes #8562

Relates

Fixes a performance issue with massive lists like sphinx-doc#8562.

jQuery.slideDown is pretty bad perf-wise since it thrashes layout
multiple times.

The 5ms animation is unnoticeable (it last less than an animation
frame!) and just burns CPU cycles unnecessarily in all browsers.

On Firefox this is specially bad because it hits a performance cliff
with list items / CSS counters
(https://bugzilla.mozilla.org/show_bug.cgi?id=1683910#c26 for all the
gory details) where it causes tons of counter recalc.

An alternative fix for the Firefox cliff would be to set overflow:
hidden on the list item, but I think removing the animation is probably
more sensible, given that as I said it's just burning CPU.

Fixes sphinx-doc#8562
@tk0miya tk0miya added this to the 3.5.0 milestone Feb 11, 2021
@tk0miya tk0miya merged commit fbe5f43 into sphinx-doc:3.x Feb 11, 2021
@tk0miya
Copy link
Member

tk0miya commented Feb 11, 2021

Thanks!

tk0miya added a commit to tk0miya/sphinx that referenced this pull request Feb 11, 2021
@emilio emilio deleted the long-result-list-fix branch February 13, 2021 03:04
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Search crashes my browser (on docs.sqlalchemy.org)
2 participants