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

Performance bottleneck in scrollbarWidth? #402

Closed
ineuwirth opened this issue Feb 7, 2018 · 7 comments
Closed

Performance bottleneck in scrollbarWidth? #402

ineuwirth opened this issue Feb 7, 2018 · 7 comments

Comments

@ineuwirth
Copy link
Contributor

Hi,

I am looking forward to seeing your opinion whether I misuse the library or have found a performance issue.

We currently use floatThead in a large static single-page documentation containing 39 tables. Simply iterating over all the tables took around 10 seconds (my workstation: Core i5, 16GB RAM).

I applied a simple optimization by skipping the tables that have a height less than $(window).height(). It reduced the time taken to ~8 seconds. Statistics about the number of rows of the remaining tables:

  • number: 29
  • avg: 19
  • min: 3
  • max: 83

Deferring the script did not help too much either because the browser hung. As rendering the page was still pretty slow I traced the code down to the suspicious scrollbarWidth() call. It is called for each table and forces reflow (darker purple bars). See:

image

If I cached the result of scrollbarWidth() and passed to the subsequent floatThead() calls then the bottleneck disappeared:

image

I have not found any workaround without changing the floatThead script itself.

JSFiddle: https://jsfiddle.net/8nzx6Lhm/

  • Chrome 64.0.3282.140 (Official Build) (64-bit)
  • Windows 10
  • jQuery 1.9.0
  • floatThead 2.0.3
@mkoryak
Copy link
Owner

mkoryak commented Feb 7, 2018 via email

@mkoryak
Copy link
Owner

mkoryak commented Feb 7, 2018 via email

@ineuwirth
Copy link
Contributor Author

Thank you for your prompt feedback. I would be happy to submit a PR however I could do it tomorrow at the earliest.

@mkoryak
Copy link
Owner

mkoryak commented Feb 7, 2018

I looked at the source, the other feature detection is all memorized already.

ineuwirth added a commit to ineuwirth/floatThead that referenced this issue Feb 12, 2018
…significantly by avoiding forced reflow in the subsequent `floaThead()` calls.

For further details please see issue mkoryak#402.
mkoryak pushed a commit that referenced this issue Feb 13, 2018
…significantly by avoiding forced reflow in the subsequent `floaThead()` calls. (#403)

For further details please see issue #402.
@mkoryak mkoryak added the solved label Feb 13, 2018
@mkoryak
Copy link
Owner

mkoryak commented Feb 13, 2018

Thanks for the PR. Looks like I have one more to merge, and after that ill cut a new release.

@mkoryak mkoryak closed this as completed Feb 13, 2018
@ineuwirth
Copy link
Contributor Author

Thank you @mkoryak

@lock
Copy link

lock bot commented Dec 10, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants