Re: [Laszlo-checkins] r13786 - openlaszlo/trunk/WEB-INF/lps/lfc/kernel/dhtml

This doesn't seem right to me:

On 2009-04-30, at 18:57EDT, max <at> openlaszlo.org wrote:

> +        ltsp._sizedomcache['lzdiv~~~' + cacheFullKey] = mdiv;

Isn't it redundant with the _sizecache now?  Won't the same mdiv have  
multiple entries in the _sizedomcache?

Re: [Laszlo-checkins] r13786 - openlaszlo/trunk/WEB-INF/lps/lfc/kernel/dhtml

There won't be multiple entries because we only create a div per style - 
see:

var mdiv = _sizecache[mdivKey];

In any case, that hash is only used by IE to prevent memory leaks...

P T Withington wrote:
> This doesn't seem right to me:
> 
> On 2009-04-30, at 18:57EDT, max <at> openlaszlo.org wrote:
> 
>> +        ltsp._sizedomcache['lzdiv~~~' + cacheFullKey] = mdiv;
> 
> Isn't it redundant with the _sizecache now?  Won't the same mdiv have 
> multiple entries in the _sizedomcache?
> 

--

-- 
Regards,
Max Carlson
OpenLaszlo.org

For Review: Change 20090430-maxcarlson-m Summary: HTML component should have a minimum size

Change 20090430-maxcarlson-m by maxcarlson <at> Bank on 2009-04-30 16:29:03  
PDT
     in /Users/maxcarlson/openlaszlo/trunk-clean
     for http://svn.openlaszlo.org/openlaszlo/trunk

Summary: HTML component should have a minimum size

Bugs Fixed: LPP-8108 - Enforce minimum size of html component

Technical Reviewer: hminsky
QA Reviewer: mdemmon

Details: test/extensions/html - Add buttons for testing invalid method  
calls and sizes

lps/components/extensions/html - Avoid registering for onx/ony in the  
target for dhtml - unused.  __updatepos() passes null x/y to  
iframemanager to skip positioning.  Width/height values are limited to  
 >= 0.

iframemanager - Add try/catch around method call to prevent bad calls  
from affecting subsequent calls to callJavascript.

Tests: test/extensions/html.lzx?lzr=dhtml runs as before, clicking  
'bad method' no longer breaks the other buttons or causes an  
exception.  Clicking 'invalid size' no longer breaks IE.

Files:
M      test/extensions/html.lzx
M      lps/components/extensions/html.lzx

For Review: Change 20090430-maxcarlson-I Summary: Raise framerate during app initialization

Change 20090430-maxcarlson-I by maxcarlson <at> Bank on 2009-04-30 15:28:22  
PDT
    in /Users/maxcarlson/openlaszlo/trunk-clean
    for http://svn.openlaszlo.org/openlaszlo/trunk

Summary: Raise framerate during app initialization

Bugs Fixed: LPP-8136 - Set the framerate to 1000 during app  
initialization

Technical Reviewer: ptw
QA Reviewer: hminsky

Details: framerate setter caches any values during init, setting the  
framerate to 1000.  init() sets the framerate back to the cached  
value.  Move onafterinit event sending to the end of init() so it can  
be used to turn profiling back on.

Tests: Startup should be slightly faster in DHTML and SWF9, where the  
framerate can be set dynamically

Files:
M      WEB-INF/lps/lfc/views/LaszloCanvas.lzs

Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20090430-maxcarlson-I.tar

Re: [Laszlo-checkins] r13786 - openlaszlo/trunk/WEB-INF/lps/lfc/kernel/dhtml

Right, what I'm saying is the same div is going to be in the  
_sziedomcache more than once, which could cause confusion.   
Personally, I don't care if that makes IE not work, but some people  
might... :P

On 2009-04-30, at 19:16EDT, Max Carlson wrote:

> There won't be multiple entries because we only create a div per  
> style - see:
>
> var mdiv = _sizecache[mdivKey];
>
> In any case, that hash is only used by IE to prevent memory leaks...
>
> P T Withington wrote:
>> This doesn't seem right to me:
>> On 2009-04-30, at 18:57EDT, max <at> openlaszlo.org wrote:
>>> +        ltsp._sizedomcache['lzdiv~~~' + cacheFullKey] = mdiv;
>> Isn't it redundant with the _sizecache now?  Won't the same mdiv  
>> have multiple entries in the _sizedomcache?
>
> -- 
> Regards,
> Max Carlson
> OpenLaszlo.org

Re: [Laszlo-checkins] r13786 - openlaszlo/trunk/WEB-INF/lps/lfc/kernel/dhtml

Look again - the key doesn't get set unless the div is being created.  I 
agree the names are confusing...

P T Withington wrote:
> Right, what I'm saying is the same div is going to be in the 
> _sziedomcache more than once, which could cause confusion.  Personally, 
> I don't care if that makes IE not work, but some people might... :P
> 
> On 2009-04-30, at 19:16EDT, Max Carlson wrote:
> 
>> There won't be multiple entries because we only create a div per style 
>> - see:
>>
>> var mdiv = _sizecache[mdivKey];
>>
>> In any case, that hash is only used by IE to prevent memory leaks...
>>
>> P T Withington wrote:
>>> This doesn't seem right to me:
>>> On 2009-04-30, at 18:57EDT, max <at> openlaszlo.org wrote:
>>>> +        ltsp._sizedomcache['lzdiv~~~' + cacheFullKey] = mdiv;
>>> Isn't it redundant with the _sizecache now?  Won't the same mdiv have 
>>> multiple entries in the _sizedomcache?
>>
>> -- 
>> Regards,
>> Max Carlson
>> OpenLaszlo.org
> 

Re: For Review: Change 20090430-maxcarlson-I Summary: Raise framerate during app initialization

Not approved yet.

I'd like to see some proof that this optimization is valid before this  
gets checked in.

Also, it seems to me that canvas.framerate ought to _always_ reflect  
the value that it is set to (that's our rule about attributes):

   x.setAttribute('y', z) =>
   x.y === z

Otherwise we get bug reports.

So, _if_ this optimization does buy us something, I'd propose doing it  
a different way:

In canvas.construct, set the runtime frame rate high (why 1000?  why  
not 1000000?  or Infinity?), and in canvas.__LZcallInit, set the  
runtime framerate to the actual canvas.framerate.

Finally, you need to be _really_ careful about changing the timing of  
an event (onafterinit).  I'm sure you will find some app that depends  
on the exixting order!

On 2009-04-30, at 19:36EDT, Max Carlson wrote:

> Change 20090430-maxcarlson-I by maxcarlson <at> Bank on 2009-04-30  
> 15:28:22 PDT
>   in /Users/maxcarlson/openlaszlo/trunk-clean
>   for http://svn.openlaszlo.org/openlaszlo/trunk

Re: For Review: Change 20090430-maxcarlson-I Summary: Raise framerate during app initialization

Re: For Review: Change 20090430-maxcarlson-I Summary: Raise framerate during app initialization

What's the best way to prove the optimization works, given the fact that 
our profiler doesn't (yet) work in swf9?

P T Withington wrote:
> Not approved yet.
> 
> I'd like to see some proof that this optimization is valid before this 
> gets checked in.
> 
> Also, it seems to me that canvas.framerate ought to _always_ reflect the 
> value that it is set to (that's our rule about attributes):
> 
>   x.setAttribute('y', z) =>
>   x.y === z
> 
> Otherwise we get bug reports.
> 
> So, _if_ this optimization does buy us something, I'd propose doing it a 
> different way:
> 
> In canvas.construct, set the runtime frame rate high (why 1000?  why not 
> 1000000?  or Infinity?), and in canvas.__LZcallInit, set the runtime 
> framerate to the actual canvas.framerate.

Sounds good.  It's forced to a maximum of 1000 to deal with Flash 
limitations.

> Finally, you need to be _really_ careful about changing the timing of an 
> event (onafterinit).  I'm sure you will find some app that depends on 
> the exixting order!

That event isn't used anywhere I know of - other than a few test 
harnesses.  It's a really subtle change that I'm sure is safe!

> On 2009-04-30, at 19:36EDT, Max Carlson wrote:
> 
>> Change 20090430-maxcarlson-I by maxcarlson <at> Bank on 2009-04-30 15:28:22 
>> PDT
>>   in /Users/maxcarlson/openlaszlo/trunk-clean
>>   for http://svn.openlaszlo.org/openlaszlo/trunk
>>
>> Summary: Raise framerate during app initialization
>>
>> Bugs Fixed: LPP-8136 - Set the framerate to 1000 during app 
>> initialization
>>
>> Technical Reviewer: ptw
>> QA Reviewer: hminsky
>>
>> Details: framerate setter caches any values during init, setting the 
>> framerate to 1000.  init() sets the framerate back to the cached 
>> value.  Move onafterinit event sending to the end of init() so it can 
>> be used to turn profiling back on.
>>
>> Tests: Startup should be slightly faster in DHTML and SWF9, where the 
>> framerate can be set dynamically
>>
>> Files:
>> M      WEB-INF/lps/lfc/views/LaszloCanvas.lzs
>>
>> Changeset: 
>> http://svn.openlaszlo.org/openlaszlo/patches/20090430-maxcarlson-I.tar
> 

--

-- 
Regards,
Max Carlson
OpenLaszlo.org

Re: For Review: Change 20090430-maxcarlson-I Summary: Raise framerate during app initialization