Explicit gave me about a 10% performance boost. I did that before I added the queue.
For layouts, the major performance boost came from the queue. I do think it’s a good idea to leave the optimization in the width and height getters to prevent reflows when client code calls those getters. I did not do a full audit of my app, but I’m pretty sure that the optimization makes a difference. Thanks, Harbs > On Mar 25, 2018, at 12:29 AM, Piotr Zarzycki <[email protected]> > wrote: > > Hi Harbs, > > As I see your major changes is adding _explicitHeight/width and this queue. > Am I understand right all ? Which one actually helps so much ? Adding > _explicitHeight/width or that queue ? > > Thanks, > Piotr > > 2018-03-24 22:04 GMT+01:00 Harbs <[email protected] > <mailto:[email protected]>>: > >> I just pushed some changes to a layout-optimization branch. >> >> I’m seeing about a 5 *times* improvement in performance with layouts and I >> don’t see any issues with the layout. It seems to me like this optimization >> was a lot more painless than I feared. >> >> Where I was seeing a layout take about 150 ms, it’s not taking about 30 >> ms. The only place I’m seeing the dreaded "Forced reflow” warning now is in >> my own code where it needs optimization. >> >> I don’t see any down-side to the code I committed, but I’ll wait for >> feedback before I merge it in. >> >> Harbs >> >>> On Mar 24, 2018, at 10:17 PM, Harbs <[email protected]> wrote: >>> >>> This is all good info. >>> >>> Here’s the problem as I see it: >>> >>> 1. Measuring width and height cause reflows if css properties were set >> and/or the Dom structure was changed. >>> 2. EVERY layout reads the host width and height before and after layout. >> Every layout (currently) subclasses LayoutBase. >>> 3. It’s pretty unlikely for any layout so not set some property, so what >> happens in #2 will cause a reflow every time unless the width and height is >> explicitly set (using a modification to the width and height getters). >>> >>> I’m looking to solve this by the following: >>> 1. Optimize the width and height getters to return the explicit values >> if they exist. In my case, this seems to yield about a 10% improvement. >> That’s not bad, but I think we can do much better. >>> 2. Delay the layout until the entire DOM structure is measured. >>> 3. The measuring might also need to be delayed until the whole structure >> is created. >>> >>> Basically, I want to make a measuring stage and then a setting stage. >> I’m not sure whether I should use requestAnimationFrame or some other >> method. The trick will be getting the measurements and the layouts executed >> in the correct order. >>> >>> Some links on the topic: >>> >>> http://blog.fogcreek.com/we-spent-a-week-making-trello- >> boards-load-extremely-fast-heres-how-we-did-it/ < >> http://blog.fogcreek.com/we-spent-a-week-making-trello- >> <http://blog.fogcreek.com/we-spent-a-week-making-trello-> >> boards-load-extremely-fast-heres-how-we-did-it/> >>> http://www.phpied.com/rendering-repaint-reflowrelayout-restyle/ >>> <http://www.phpied.com/rendering-repaint-reflowrelayout-restyle/> < >> http://www.phpied.com/rendering-repaint-reflowrelayout-restyle/ >> <http://www.phpied.com/rendering-repaint-reflowrelayout-restyle/>> >>> http://wilsonpage.co.uk/preventing-layout-thrashing/ >>> <http://wilsonpage.co.uk/preventing-layout-thrashing/> < >> http://wilsonpage.co.uk/preventing-layout-thrashing/ >> <http://wilsonpage.co.uk/preventing-layout-thrashing/>> >>> >>> I’m planning on spending time on this tomorrow. I’ll see what I can come >> up with… >>> >>> Harbs >>> >>>> On Mar 23, 2018, at 7:11 PM, Alex Harui <[email protected] >>>> <mailto:[email protected]> >> <mailto:[email protected] <mailto:[email protected]>>> wrote: >>>> >>>> Hi Harbs, >>>> >>>> For sure, it would be great for you to take a look at layout. >>>> >>>> Besides what Peter wrote below, there are other things to consider: >>>> >>>> 1) Like Flex, Royale layouts assume that (when necessary) parents size >>>> their children. That just makes sense for responsive apps that respond >> to >>>> the size of the application window. >>>> 2) Like Flex, Royale's UIBase has APIs that "temporarily" set a size. >>>> Setting width/height also sets explicitWidth/explicitHeight, but the >>>> setWidth/setHeight/setWidthAndHeight calls only set the width/height >> and >>>> do not set explicitWidth/explicitHeight in order for that layout pass to >>>> set the size but not have the explicitWidth/Height factored into the >> next >>>> layout pass. >>>> 3) The code snippet you posted I'm pretty sure is used for when a >> child's >>>> size changes and the parent's size should then change. As Peter said, >>>> some components are sized to content and the layout should be responsive >>>> to the content. >>>> >>>> But what I think is most important is really what the DOM looks like. >> The >>>> only goal for Royale layouts for JS is to set up the DOM. In theory, >> the >>>> default VerticalLayout and HorizontalLayouts do not measure anything nor >>>> do they set the width and height on the children. They merely set the >>>> display style to block or inline-block. Then it is up to the browser to >>>> decide when/how to layout. Ideally, our JS code is creating widgets and >>>> setting display styles in the most optimal way to create the same DOM >> you >>>> would by hand with an HTML editor. >>>> >>>> So, for your app, the first question is: did Royale set up the DOM in >> the >>>> best possible way? I think you said there was a fair amount of absolute >>>> positioning in your app. If the Royale absolute positioning layout is >> not >>>> allowing the construction of the DOM in the best possible way, the next >>>> question is then: should the current layout algorithm be changed because >>>> it is just plain wrong, or should you create a different absolute >>>> positioning layout that is optimized for the kinds of DOM sub-trees you >>>> like to use? Or should you be revising your app to use a layout based >> on >>>> FlexBox or CSSGrid? >>>> >>>> So, it might turn out that BasicLayout is always going to be slow but it >>>> will handle just about any absolute positioning from both top-down >>>> percentage sizing as well as sizing to content of children, but that new >>>> layouts should be created that drastically reduce the number of folks >> that >>>> need to use BasicLayout in production. >>>> >>>> My 2 cents, >>>> -Alex >>>> >>>> On 3/23/18, 7:25 AM, "Peter Ent" <[email protected] >>>> <mailto:[email protected]> <mailto: >> [email protected] <mailto:[email protected]>>> wrote: >>>> >>>>> The code looks familiar, but I'm not 100% sure. >>>>> >>>>> The trick with layouts is that there are the following things to >> consider: >>>>> >>>>> If everything has an explicit width/height, it should be pretty easy. >>>>> >>>>> The percentWidth/height when present changes the _width, _height >> without >>>>> changing the explicit size. This allows a layout to work again on the >>>>> percent value and calculate a new size. If the explicit sizes are set, >>>>> then the next layout pass will take the explicit over the percent >> (setting >>>>> explicit sizes sets the percent sizes to NaN and vice-versa) and what >> was >>>>> once 50% will now be fixed in size. >>>>> >>>>> The tricky part - for me anyway - is when an item has no size set. Then >>>>> its supposed be sized by its content. Labels and Buttons sized by their >>>>> text or icons while containers are sized by their children and so >> forth. >>>>> >>>>> Let's say you have one container nested inside of another and in the >> inner >>>>> most container is a single button. The outer most layout cannot >> determine >>>>> its container-child's size because it doesn't have one yet. The inner >>>>> container needs to get the size of its children (the button) and that >> then >>>>> becomes its size. But it should not be setting the explicit sizes. That >>>>> allows the button to be resized which means its container becomes >> bigger >>>>> which means the outer container becomes bigger. Once you set those >>>>> explicit sizes, then its game over - use case #1 essentially. >>>>> >>>>> So - I think that code has to do with determining size-by-content and >> so >>>>> isLayoutRunning was created to prevent a recursion. >>>>> >>>>> For me, working with Flex and Royale - determining the size of >> something >>>>> has been one of the biggest challenges. >>>>> >>>>> ‹peter >>>>> >>>>> On 3/23/18, 7:46 AM, "Harbs" <[email protected] >>>>> <mailto:[email protected]> <mailto: >> [email protected]>> wrote: >>>>> >>>>>> I¹m working on making layouts more efficient. Currently, there¹s lots >> of >>>>>> setting and reading of width and height which causes many browser >>>>>> reflows. While profiling performance in my app, reflows is a major >>>>>> bottleneck. In one area, it¹s taking about 150ms (on my I7 2.8 Ghz >>>>>> MacBook Pro ‹ on tablets it¹s painfully slow) to execute on area of >>>>>> layout. Almost all of that time is being spent measuring with and >> height >>>>>> of components. The width and height getters trigger reflow because >>>>>> properties are recursively set in layout. >>>>>> >>>>>> I was able to get about a 10% improvement by optimizing the width and >>>>>> height getters to return the explicitWdith and explicitHeight if set. >> It >>>>>> looks to me like almost all of this bottleneck could be eliminated by >>>>>> delaying the property setting until after the measurements are done. >> I¹m >>>>>> working on doing that, but I have a question: >>>>>> >>>>>> LayoutBase.performLayout has the following code: >>>>>> >>>>>> // check sizes to see if layout >> changed the size or not >>>>>> // and send an event to re-layout >> parent of host >>>>>> if (host.width != oldWidth || >>>>>> host.height != oldHeight) >>>>>> { >>>>>> isLayoutRunning = true; >>>>>> host.dispatchEvent(new >> Event("sizeChanged")); >>>>>> isLayoutRunning = false; >>>>>> } >>>>>> >>>>>> Under what circumstances does this code get executed? This appears to >> be >>>>>> causing a recursive layout. Can I assume that there will be an >>>>>> explicitWidth/height when this will be executed? >>>>> >>>> >>> >> >> > > > -- > > Piotr Zarzycki > > Patreon: *https://www.patreon.com/piotrzarzycki > <https://www.patreon.com/piotrzarzycki> > <https://www.patreon.com/piotrzarzycki > <https://www.patreon.com/piotrzarzycki>>*
