Opened 6 years ago

Closed 3 years ago

#17307 closed defect (patchwelcome)

Tree: performance issue caused by _onItemChildrenChange() when there are huge amount of child nodes

Reported by: wangfcdl Owned by: bill
Priority: undecided Milestone: 1.13
Component: Dijit Version: 1.8.0
Keywords: Cc:
Blocked By: Blocking:

Description

Some of our product codes called function "newItem" of "dojo.data.ItemFileWriteStore?" to create a tree view. Since our dojo version is upgraded from 1.0 to 1.5, we found serious performance problem when browsing the tree in IE. After investigating, we found that it is cause by the logic of "_onItemChildrenChange" of Tree.js that responds to every new item event. It removes all existing child nodes and add the new node list again. So if there are n child nodes, there would be n*(n+1)/2 times calling on adding child nodes, while in dojo 1.0, with "_onNewItem" responding to every new item event the times is n. The performance downgrades when n become large. The latest version of our product uses dojo 1.8, and it has the same performance issue.

Change History (5)

comment:1 Changed 6 years ago by bill

Component: GeneralDijit
Milestone: tbd2.0
Owner: set to bill
Summary: Performance issue caused by "_onItemChildrenChange" of Tree.js when there are huge amount of child nodesTree: performance issue caused by _onItemChildrenChange() when there are huge amount of child nodes

I see, seems like a problem w/the API between the model and the Tree, specifically that _onItemChildrenChange() just gets the new list of children rather than the description of what changed.

You should be using ObjectStoreModel? rather than the deprecated TreeStoreModel? (and dojo/store rather than the deprecated dojo/data), but think it would hit the same issue.

I'll mark this as 2.0 for now. If you want to try patching it yourself though I'd be happy to look over the patch and advise on it.

comment:2 Changed 6 years ago by wangfcdl

Thanks for your feedback, Bill. We may have to try patching it ourselves. Do you know why it changes to remove existing nodes and then add the new node list instead of adding the new node when there is a new item event? Is it for fixing a defect or for other consideration? Thanks.

comment:3 Changed 6 years ago by wangfcdl

One more thing, Firefox seems not suffered a lot with this DOJO change. In our test, expanding a parent node with 200 child nodes takes 5-7 sec with Firefox, but takes 60-75 sec with IE.

comment:4 in reply to:  2 Changed 6 years ago by bill

Replying to wangfcdl:

Thanks for your feedback, Bill. We may have to try patching it ourselves. Do you know why it changes to remove existing nodes and then add the new node list instead of adding the new node when there is a new item event? Is it for fixing a defect or for other consideration? Thanks.

I guess it's historical. With the old dojo.data API (as opposed to the new dojo.store API), the onNew() notification for a new item lists the new item's parent, and theoretically the new value of the parent item's "children" attribute. So it will theoretically say that node XYZ now has a children list of [A, B, C, D], as opposed to the old child list of [A, B, C]. But I'm not sure if that's always reported or not. For example, the case when the child points to the parent rather than the parent listing it's children. So with the old dojo.data API it seemed safer to just call getChildren() on the parent, to get the new list of children.

Replying to wangfcdl:

One more thing, Firefox seems not suffered a lot with this DOJO change. In our test, expanding a parent node with 200 child nodes takes 5-7 sec with Firefox, but takes 60-75 sec with IE.

OK that sounds unrelated to this ticket, which is about changing the data in the store, not about opening/closing tree nodes.

comment:5 Changed 3 years ago by dylan

Milestone: 2.01.12
Resolution: patchwelcome
Status: newclosed

Given that no one has shown interest in creating a patch in the past 2+ years, I'm closing this as patchwelcome.

Note: See TracTickets for help on using tickets.