iron-fit-behavior icon indicating copy to clipboard operation
iron-fit-behavior copied to clipboard

Perf degradation due to calls to window.getComputedStyle in attached

Open benoitjchevalier opened this issue 8 years ago • 5 comments

In attached we remember if the element is RTL or not: this._isRTL = window.getComputedStyle(this).direction == 'rtl';

Even though this is designed as too avoid having to make recurrent call to getComputedStyle attached can be called again when moving the element around in the dom, performing an unneeded calculation/layout.

In our case the performance hit actually shows when moving an item around having 40 subitems which all get "attached".

benoitjchevalier avatar Feb 16 '17 01:02 benoitjchevalier

getComputedStyle added for the first render +60ms. May be there is another way to identify LTR/RTL. For example providing as property and setting by default to false or calculating in setTimeout. Why this issue is still opened?

madiyetov avatar Oct 04 '17 14:10 madiyetov

This issue causes ~20+% rendering performance degradation in the real app, please see: https://jsfiddle.net/xh0q7oqv/

image

All reds you see - forced reflow @ iron-fit-behavior.html:262

PolymerElements/paper-dropdown-menu#284

mkorenko avatar Apr 11 '18 19:04 mkorenko

Chiming in a little late - but we have the same issue in our app. We are including lots of elements which use iron-fit-behavior and each one is performing this calculation. Can you do this as a scoped variable in iron-fit-behavior so it is calculated once upon load.

chiefcll avatar Jul 19 '18 20:07 chiefcll

Same here. Performance degradation because of many elements triggering the calculation.

krozycki avatar Jul 22 '18 19:07 krozycki

Maybe a decent solution would be to add a new feature, along the lines of what @stat92 mentioned. Add an isRtl property, if the property has been set externally then skip the getComputedStyle in attached, otherwise keep the same behavior. That way it's backward compatible and only require a minor bump, people having issue with performance regarding this specific scenario can fix it and other people can keep using it as before.

Not sure how easy it would be to get some Polymer people to have a look at it but might be worth issuing a PR and tagging some of them

benoitjchevalier avatar Jul 23 '18 17:07 benoitjchevalier