So, can we be sure that in the optimization for __processHiddenParents that there is going to be an __LZdivdisplay entry for every sprite? Maybe a better optimization would be to use the cache to see if the parent is not currently being displayed, and if not, display it, and add it to a list of sprites to be reverted? I'm thinking that it is probably more inefficient to unnecessarily set the display style on sprites that don't need it.
Otherwise, approved. On 2010-01-27, at 19:58, Max Carlson wrote: > Here's the relevant bit for this change- sorry, should have made it clearer: > > Unify caches, clarify sprite property setting in applyCSS(). Add > show_img_before_changing_size quirk, restrict to IE-only. Optimize > __processHiddenParents, using cached display values to restore div > state. Flatten __csscache key names, e.g. '__LZdivdisplay'. > > Regards, > Max Carlson > OpenLaszlo.org > > On 1/27/10 4:51 PM, P T Withington wrote: >> Can you summarize what you did to update this, and perhaps explain how they >> addressed my previous comments? It would make review easier, rather than me >> having to start from scratch. >> >> On 2010-01-26, at 18:10, Max Carlson wrote: >> >>> Change 20100125-maxcarlson-p by maxcarl...@bank on 2010-01-25 16:07:08 PST >>> in /Users/maxcarlson/openlaszlo/trunk-clean >>> for http://svn.openlaszlo.org/openlaszlo/trunk >>> >>> Summary: UPDATED: Improve DOM performance >>> >>> Bugs Fixed: LPP-8734 - Improve DHTML DOM performance >>> >>> Technical Reviewer: ptw >>> QA Reviewer: hminsky >>> >>> Details: Add caching for CSS properties, to prevent spurious updates. Note >>> that I started by using applyCSS() for _all_ CSS properties, then noticed >>> that width, height display and overflow had by far the most cache hits. >>> Then, I removed applyCSS() for other CSS properties. This saves ~1100 >>> spurious DOM updates. Unify caches, clarify sprite property setting in >>> applyCSS(). Add show_img_before_changing_size quirk, restrict to IE-only. >>> Optimize __processHiddenParents, using cached display values to restore div >>> state. Flatten __csscache key names, e.g. '__LZdivdisplay'. >>> >>> LzSprite - Add applyCSS() to apply CSS properties to a div. >>> >>> LzSprite,LzTextSprite,LzInputTextSprite - Init __csscache at construct >>> time. Use applyCSS() to apply width, height, display and overflow >>> properties. Use __csscache instead of reading the div's style property for >>> checking the state of those properties. >>> >>> LzTextSprite,LzInputTextSprite - Cache this.className property when set, >>> for use with text measurement. >>> >>> Tests: I see the following when starting webtop DHTML with and without this >>> patch applied: >>> Before : Profile (7938.758ms, 219848 calls) >>> After: Profile (7449.889ms, 229325 calls) >>> >>> Files: >>> M WEB-INF/lps/lfc/kernel/dhtml/LzSprite.js >>> M WEB-INF/lps/lfc/kernel/dhtml/LzTextSprite.js >>> M WEB-INF/lps/lfc/kernel/dhtml/LzInputTextSprite.js >>> >>> Changeset: >>> http://svn.openlaszlo.org/openlaszlo/patches/20100125-maxcarlson-p.tar >>> >>> _______________________________________________ >>> Laszlo-reviews mailing list >>> [email protected] >>> http://www.openlaszlo.org/mailman/listinfo/laszlo-reviews >> > _______________________________________________ > Laszlo-reviews mailing list > [email protected] > http://www.openlaszlo.org/mailman/listinfo/laszlo-reviews _______________________________________________ Laszlo-reviews mailing list [email protected] http://www.openlaszlo.org/mailman/listinfo/laszlo-reviews
