LzDelegate:
+ if (eventSender !== this.c) {
+ var _dels:Array = this.c['__delegates'];
+ if (_dels == null) {
+ this.c.__delegates = [ this ];
+ } else {
+ _dels.push( this );
+ }
+ }
1) This will create multiple entries for each delegate, you should add a
boolean flag to track whether the delegate was already added to the
__delegates array to prevent more than one entry.
2) What about the issue with the swf8 kernel using LzDelegates?
LzDelegate:
+public function destroy ():void {
+ this.unregisterAll();
+ this.c = null;
+ this.m = null;
+}
Please add "@access private" to hide the function from the docs - can
you remind me why functions are added to the docs by default?! There is
a JIRA issue to switch this default behaviour, isn't it?
LzNode:
+ for ( var i = dels.length - 1; i >= 0; i-- ){
+ if (dels[i].c) {
+ dels[ i ].destroy();
+ }
}
Won't work for LzDataAttrBind - there is no "c" attribute, so the
if-test will fail.
On 3/23/2010 11:29 PM, Max Carlson wrote:
Change 20100314-maxcarlson-0 by maxcarl...@bank on 2010-03-14 00:54:51 PST
in /Users/maxcarlson/openlaszlo/trunk-clean
for http://svn.openlaszlo.org/openlaszlo/trunk
Summary: UPDATE 4: Automatically remove invalid delegates in
LzEventable.destroy()
Bugs Fixed: LPP-8822 - Improve layout performance
Technical Reviewer: ptw
QA Reviewer: hminsky
Release Notes: It is no longer necessary to manually track and release
delegates whose target is 'this' to prevent memory leaks. This is handled
automatically by the event system.
Details: Updated to address Tucker's comments:
1) In LzEventable, __removeDelegates does not need to be a separate function
any more, does it?
I asked laszlo-user about the need for layout.releaseLayout(). Until that's
resolved, __removeDelegates is required :(.
2) LaszloEvents.lzs#102: deletes -> deleted
Done.
3) Can we poll laszlo-user to see if anyone is using `releaseLayout*`?
Done.
4) Perhaps rename __LZdelegates to make it more obvious that it is now _only_
for tracking delegates that implement contraints?
Done.
Issues that still need to be addressed:
1) Compiler error in swf10 due to return before super call in LzDelegate
constructor. (You can't conditionally execute a super call in a constructor.
In ES Harmony I think the only thing you will be able to do is specify which,
if any, of your args are passed to your super.)
Fixed.
2) LzEventable should not be peeking inside an LzDelegate and smashing it's `c`
and `m` properties (see issue #3 for why). You should create a new API for
LzEventable (it might be called `destroy`) that does the work of unregistering
and neutering it, and call that from LzEventable.
Done. I added a destroy() API to LzDataAttrBind for symmetry - see
LzNode.destroy().
3) We still don't quite have this tracking right. We don't need to create an
entry for _every_ delegate, and I think we have the potential of adding a lot
of useless overhead if we don't minimize this tracking to the minimum required.
We only have to track delegates that are registered on events from another
node. So I think that the decision whether or not to add a new delegate to
__delegates in LaszloEvents should be in `register`, and only when the event
sender is not the delegate context.
Done.
4) LaszloeEvents#233
`if (this.c&& this.c['__LZdeleted']) {`
This seems wrong. When you "destroy" a delegate in LzEventable, you set `c` to
null, so this test will fail and you could end up adding a destroyed delegate to an
event. I think the logic should be:
`if ((this.c == null) || this.c['__LZdeleted']) {`
Done.
LzNode,LzState - Rename __LZdelegates -> __LZconstraintdelegates, shorten
array lookups where possible, use LzDelegate.destroy() instead of unregisterAll().
LaszloEvents - Add check/warning when a delegate is created for an invalid
context. Move __delegates tracking to register() and only add LzEventables
that are registering for a different context. Add warnings for unneeded
unregisterAll() calls, but comment them out due to too many warnings. Add
destroy() API that neuters and unregisters the delegate.
LzDataAttrBind - Add destroy() API for symmetry with LzDelegate.
LzDatapath - Use super.destroy() for efficiency.
LaszloView - releaseLayouts() now calls layout.destroy().
LzState - Rename __LZstatedelegates -> __LZstateconstraintdelegates.
Otherwise, the same as before:
LzInputText,LzAnimatorGroup,LzDatapointer,LzDataset - Remove extra delegate
unregistration as it's now handled by the event system.
LzEventable - Add _delegates array to hold delegates that have a context
pointing to this object. Clean out delegate contexts in destroy().
LaszloEvents - LzDelegate registers on its contexts' _delegates array.
LaszloLayout,resizelayout - don't track delegates anymore.
Tests: Run modified leak-components test in swf10 - the memory monitor in the
upper left shows much less leakage with this patch: you can click the monitor
to force garbage collection. Also run smokecheck.
Files:
M test/components/lz/leak-components.lzx
M WEB-INF/lps/lfc/core/LzNode.lzs
M WEB-INF/lps/lfc/core/LzEventable.lzs
M WEB-INF/lps/lfc/views/LzInputText.lzs
M WEB-INF/lps/lfc/views/LaszloView.lzs
M WEB-INF/lps/lfc/helpers/LzState.lzs
M WEB-INF/lps/lfc/events/LaszloEvents.lzs
M WEB-INF/lps/lfc/controllers/LzAnimatorGroup.lzs
M WEB-INF/lps/lfc/controllers/LaszloLayout.lzs
M WEB-INF/lps/lfc/data/LzDatapointer.lzs
M WEB-INF/lps/lfc/data/LzDataset.lzs
M WEB-INF/lps/lfc/data/LzDatapath.lzs
M WEB-INF/lps/lfc/data/LzDataAttrBind.lzs
M docs/src/developers/delegates.dbk
M lps/components/rpc/ajax.lzx
M lps/components/extensions/html.lzx
M lps/components/utils/layouts/resizelayout.lzx
M lps/components/utils/replicator/replicator.lzx
M lps/components/av/rtmpstatus.lzx
M lps/components/base/basetabelement.lzx
M lps/components/base/basescrollbar.lzx
M lps/components/base/basecomponent.lzx
M lps/components/base/basewindow.lzx
M lps/components/base/submit.lzx
M lps/components/base/basetrackgroup.lzx
M lps/components/base/basedatacombobox.lzx
Changeset:
http://svn.openlaszlo.org/openlaszlo/patches/20100314-maxcarlson-0.tar
