Not approved yet.

I find it really confusing that we have `setCSS` and `applyCSS`, that they both 
use the same cache, but one indexes the cache just by property and the other by 
div and property (hence the top-level of the cache is a mix of property and div 
names).  Is there any way we can unify this?

Line 3059 in LzSprite:

    cache[divname][name] = this[divname].style[name] = value;

is too clever by half.  It took me forever to see that you were finally 
actually updating the CSS in the DOM here.  Can you either split this into two 
updates or put in a comment so dummies like me can see what is happening?

---

Possible improvement:  I wonder if there is some way we could batch up CSS 
updates even further, say by _not_ updating the DOM style directly in applyCSS; 
instead, accumulate the cache misses and apply them in an idle co-routine 
(i.e., a timer with 0 delay)?

On 2010-01-25, at 19:25, 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: 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.
> 
> 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

Reply via email to