Opened 6 years ago

Closed 6 years ago

#16660 closed enhancement (fixed)

[patch][ccla] Reduce call stack in _ContentPaneResizeMixin#_layoutChildren

Reported by: Kenneth G. Franqueiro Owned by: bill
Priority: undecided Milestone: 1.9
Component: Dijit Version: 1.8.3
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by Kenneth G. Franqueiro)

A customer identified a potential improvement to the _layoutChild method of _ContentPaneResizeMixin which has potential to significantly reduce the depth of call stacks (which can reduce memory consumption particularly in old IE).

The change involves changing from using array.forEach to using a straight for loop. Because the code in question is within _layoutChildren which is called by resize, this has potential to reduce the number of function calls not only on an individual ContentPane?, but also any nested ContentPanes? underneath.

Attaching a patch (under SitePen? CCLA). This patch was created from the 1.7 branch (since the customer is on 1.7), but will also cleanly apply to 1.8 and trunk.

Attachments (1)

layoutchildren.diff (688 bytes) - added by Kenneth G. Franqueiro 6 years ago.

Download all attachments as: .zip

Change History (6)

Changed 6 years ago by Kenneth G. Franqueiro

Attachment: layoutchildren.diff added

comment:1 Changed 6 years ago by bill

My guess is that

while(child = children[i++]){ ... }

would be even faster.

It's hard to believe either change is significant though, nor would I say that the change reduces recursion. But if it actually does reduce memory usage in a measurable way we should do it.

comment:2 Changed 6 years ago by Kenneth G. Franqueiro

Yeah, your loop would probably work too.

The idea is that it no longer has to call forEach, nor would forEach perform additional calls to a callback, so it should be 2 fewer levels of recursion for each recursive level of ContentPane resizes.

Admittedly I was not able to see a noticeable performance difference myself (I tried, with a drastically unrealistic amount of nesting), and it's hard to really nail memory consumption since ostensibly memory would bounce back to normal the moment the resize operation completes.

comment:3 Changed 6 years ago by Kenneth G. Franqueiro

Description: modified (diff)
Summary: [patch][ccla] Reduce recursion in _ContentPaneResizeMixin#_layoutChildren[patch][ccla] Reduce call stack in _ContentPaneResizeMixin#_layoutChildren

After talking with Bill on IRC, I'm realizing that what I mean isn't technically reduced recursion, but reduced call stack depth. Changing title to hopefully clarify.

comment:4 Changed 6 years ago by bill

Milestone: tbd1.9
Status: newassigned
Type: defectenhancement

OK, I'll switch it to a native loop in case it makes a difference.

comment:5 Changed 6 years ago by bill

Resolution: fixed
Status: assignedclosed

In [30679]:

use native loop instead of array.forEach() to fix alleged memory consumption issue in IE, fixes #16660 !strict

Note: See TracTickets for help on using tickets.