So, here, I am _not_ creating a method closure. Instead, I am introducing the overhead of a runtime method lookup. In a better compiler, we'd have a macro for the draw method and just call it for the two cases. That would avoid the code skew, the runtime method lookup, and method closures. We don't have that, so I am taking the runtime lookup as the least evil path.
Gory details: method closures can be a real problem for GC's. They have been, since forever, because when you have a language that automatically creates closures for you as necessary, it is easy to accidentally be creating closures in a loop and the programmer is totally unaware of it. It makes we worry a bit about ES6, because it intends to support method closures too. As an example of how they can kill the GC, consider this case you were proposing. Each time you call the roundRect method, it would create a closure for `context.curveTo`, call it possibly 4 times, and then drop it. If you have a _really_ _smart_ compiler, it would realize that this closure has only dynamic extent (only lives for the length of the function call, is not captured anywhere, is not passed out of the frame upwards), and it would stack-allocate this closure. But that is _really_ hard to do, especially with a language like Javascript, where you have horrible things like `eval` and `arguments.callee` which essentially break any chance the compiler has of figuring that out. So, most likely, that closure is allocated on the heap. If you are lucky, the GC is a generational GC and that closure, because it dies very quickly, is quickly reclaimed. But, nonetheless, it does cause some GC work. Now, imagine that roundrect method is called many, many, times to draw your UI elements. The same way the compiler could not tell that the closure had dynamic extent, there is no way for it to tell that it could potentially re-use the closure (Javascript semantics say you can tell the difference between two instances of the same closure), so it has to make a new one on each call... It's easy to see how this could get out of hand. Hence, I think the runtime method lookup is a much smaller evil. When we find out that is really a bottleneck, we can 'hand-macro' the code. But, I don't want to do that while the code is still in flux, which it is bound to be while I sort out the rest of the box-model details. On 2010-11-12, at 09:55, Max Carlson wrote: > I see. One of the leading performance issues Henry and i saw from profiling > swf10 looked to be method closures, at lest from a gc/memory churn > standpoint. Approved for now! > > On Nov 12, 2010, at 6:11 AM, P T Withington <[email protected]> wrote: > >> The line you are complaining about is unchanged from the original code. >> What is it that is troubling about it? >> >> Only as3 supports method closures, so the only way you could avoid saying >> `context[curvemethod]` would be to duplicate the code. At this point in >> time, it seems better to keep a single implementation. If it turns out this >> is a performance bottleneck, we can change it later. >> >> On 2010-11-12, at 04:56, Max Carlson wrote: >> >>> This line is troubling: >>> var curvemethod = context['curveTo'] ? 'curveTo' : 'quadraticCurveTo'. >>> >>> Can you look up the method reference once ahead of time instead of >>> computing its name and looking it up, e.g. context[curvemethod]? >>> Perhaps use an if ($dhtml) {...} else {...} clause that stores a reference >>> to the method once? >>> >>> Otherwise approved! >>> >>> On 11/11/10 1:35 PM, P T Withington wrote: >>>> Change ptw-20101111-LWK by [email protected] on 2010-11-11 16:26:03 EST >>>> in /Users/ptw/OpenLaszlo/trunk-3 >>>> for http://svn.openlaszlo.org/openlaszlo/trunk >>>> >>>> Summary: Generalize rectangle drawing to support oval borders, CSS >>>> defaulting >>>> >>>> Bugs Fixed: LPP-9484 Strokes in drawbutton shoulld be stylable (partial) >>>> >>>> Technical Reviewer: [email protected] (pending) >>>> QA Reviewer: [email protected] (pending) >>>> >>>> Details: >>>> >>>> Made a new method, roundrect, that takes separate horizontal and >>>> vertical radii for the corners. All radii must be supplied, >>>> defaulting should be done at a higher level. Uses the CSS >>>> algorithm for scaling radii to fit small boxes. This low-level >>>> method is needed to be able to draw different width borders that >>>> smoothly transition around corners. >>>> >>>> Made rect default its radii arguments according to the CSS >>>> defaulting algorithm and call roundrect. >>>> >>>> Tests: >>>> smokecheck, is there a test for rect? >>>> >>>> Files: >>>> M WEB-INF/lps/lfc/kernel/LzKernelUtils.lzs >>>> >>>> Changeset: >>>> http://svn.openlaszlo.org/openlaszlo/patches/ptw-20101111-LWK.tar >>
