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

Bad performance with Chrome and sibling selector #4322

Closed
cguglielmo opened this Issue Mar 14, 2019 · 3 comments

Comments

Projects
None yet
2 participants
@cguglielmo
Copy link

commented Mar 14, 2019

Hi there

We experienced a weird performance issue with jQuery 3.3.1 which does not occur with jQuery 2.2.4. As far as we can tell, only Chrome is affected.

The problem is, that the following code forces the browser to recalculate its style.
$elem.css('width', 'auto'); $elem.css('display', 'inline-block');

If this is done in a loop let's say for 1000 elements, the browser will recalculate the style a 1000 times! But, it only seems to happen if there is a CSS rule using a sibling selector for the container element of the 1000 nodes. The CSS rule can even be empty, but the rule has to be in a separate style sheet. If the style is embedded in the HTML document, it works fine...

We could track it down to the following line:
isBorderBox = jQuery.css( elem, "boxSizing", false, styles ) === "border-box",

Replacing the above line with the following line fixes the problem:
isBorderBox = extra && jQuery.css( elem, "boxSizing", false, styles ) === "border-box",

Reading the box-sizing attribute obviously forces the browser to recalculate the style. But honestly, we don't know what this patch could break. It looks like the code is only necessary for IE 9? At least according to issue #3699.

An example to reproduce the issue is attached.
jQuery Perf.zip

@mgol

This comment has been minimized.

Copy link
Member

commented Mar 14, 2019

Thanks for the report! We fixed a similar issue in #4187 but we missed this one.

We will most likely not be able to avoid layout thrashing in IE but we might be able to do that in other browsers.

@cguglielmo You can check whether your change breaks something by running npm test in the repository.

@mgol

This comment has been minimized.

Copy link
Member

commented Mar 18, 2019

PR: #4325

@mgol mgol closed this in #4325 Mar 18, 2019

mgol added a commit that referenced this issue Mar 18, 2019

@mgol mgol added this to the 3.4.0 milestone Mar 18, 2019

@mgol

This comment has been minimized.

Copy link
Member

commented Mar 18, 2019

@cguglielmo This has been fixed. You can double-check with our dev version, https://code.jquery.com/jquery-git.js (just don't use it in production!). I checked your test case & it no longer does layout thrashing.

@GulajavaMinistudio GulajavaMinistudio referenced this issue Mar 20, 2019

Merged

adjust MaybeUninit API to discussions #78

0 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.