[Solved] ShellTreeView Node Expand: why second expand is slower than first one?

[d7_2_laz]

Coincidentally seen (Win x64): open (eg. via the ShellView demo) a highly populated folder (eg. Windows\WinSxS).
Collapse it. Expand it again ... appears to be much slower than the first expand. Why?
Would have expected that the second expand runs instantly, as the node is already populated.
Why the node is populated each time? (I assume there is a reason)

Changed simply for testing CanExpand  with instant second expand:

Code: Pascal  [Select][+][-]

1. function TCustomShellTreeView.CanExpand(Node: TTreeNode): Boolean;
2. var
3.   OldAutoExpand: Boolean;
4. begin
5.   Result:=inherited CanExpand(Node);
6.   if not Result then exit;
7.   OldAutoExpand:=AutoExpand;
8.   AutoExpand:=False;
9.   BeginUpdate;
10.   try
11.     Result := True;
12.     if Node.Count = 0 then begin     // Added simply for test
13.         //Node.DeleteChildren;       //  Not becessary regarding the condition
14.         Result := PopulateTreeNodeWithFiles(Node, GetPathFromNode(Node));
15.     end
16.     else  Result := True;           // Added
17.     AutoExpand:=OldAutoExpand;
18.   finally
19.     EndUpdate;
20.   end;
21. end;

[wp]

I had thought that child nodes would be destroyed when their parent is collapsed, in order to save memory load. But I cannot find anything related to "Collapse" in the ShellTreeView code - so, probably I am wrong, and your modification would be a solution.

On the other hand, why not implement that "Free-on-collapse" thing? Would there be a disadvantage in having it?

[d7_2_laz]

Hi wp, i'm simply not yet deep enough inside the Shell tools for to understand
- why at all a population might be triggered by a CanExpand query
- and why at all nodes might be cleared or childs destroyed when a node is collapsed.
To my simple understanding maybe intuition a node collapsed is a potential candidate to be re-opened, so i never would clear it when being collapsed.
I assume there is a good reason to do so, but i don't know it. Simply missing of info on my side. What's the benefit to clear? Save memory? Never did run into a bottleneck here.

Beyond intuition, most other file tools i know about re-open (re-expand) nodes instantly and that wuld be my own expectation too.

A nice side-effect i could imagine is that a re-populated node will on-the-fly catch all modifications of the folder subtree needed from actions outside.
But imo that would be rather a question of how to deal with file change monitors.

I was simply really surprised that a subsequent expand takes (felt:) the doubled time of the first one (may be internally the population is done twice?).
The disadvantage of a "Free-on-collapse" in my eyes: why not keep information that is already there but destroy it? Would lead to much redundant work.

I don't think the nodes are cleared intentionally (eg. at collapse). It's simply that they are ckeared and re-built each time a node is re-opened again. (no memory save advantage could be seen here).

For to avoid misunderstanding: i feel the shell tools are excellent - that's why I want to use them more and more  for new things.

[d7_2_laz]

I experimented a while using the change mentioned above (within CanExpand) and did not find any negative indication why it should not be applied.

So i did open a issue note on that:
https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/40022

Supplementary remark: i'd expect that when selecting a subnode "Sub1" beyond parent node "Parent1", that when i collapse Parent1 by click on the expand sign, and re-expand it again, the previous selection marker on Sub1 is preserved.
Using the change, it stays preserved. By the current behaviour (= sub nodes cleanup) it goes lost. Not ok. Please keep my selection!

[GetMem]

@d7_2_laz
From the bugreport:

when re-expanding the node, this is done instantly without clearing/repopulating the node
No more node population is done again redundantly. And certainly not it should take twice the time.

I'd expect that an already populated node can be re-expanded instantly.

What if the collapsed node's children are physically deleted from the hard drive while the node is collapsed? Isn't the (re)read on expand intentional? I wouldn't call it redundant. Maybe you can introduce a new flag, when set to true,  the disk changes are ignored for an already loaded node. 

[d7_2_laz]

I'dthought about that as above

A nice side-effect i could imagine is that a re-populated node will on-the-fly catch all modifications of the folder subtree needed from actions outside.
But imo that would be rather a question of how to deal with file change monitors.

Of course it makes sense to take care about external done file changes (implying fle change monitors). But then - consequently,  amd not only at one single place (a re-opened node). What about siblings of the parent node, if a folder is deleted: why should they stay within the tree, whereas a re-expanded node should respect duch changes?
At this very basic level external file and folder changes necessarily are not yet in scope.

[d7_2_laz]

Additionally.if you have a dirtree - filelist twin pair, you re-expand a node and expect to see folder changes hrtr,  then one necesarily needs to re-read the listview's content completely too, otherwise dirtree and listview would go out of sync.
Expand = reread whole subtree. plus reread whole lfile - listview?

[wp]

I think I'll add an ExpandCollapseMode property:

Code: Pascal  [Select][+][-]

1. type
2.   TExpandCollapseMode = (
3.     ecmDefault,          // Current behaviour (--> default)
4.     ecmKeepExpanded,     // Do not clear children of already-expanded, but collaped nodes (d7-2-laz's code)
5.     ecmCollapseAndClear  // Clear children when node is collapsed
6.   );

[d7_2_laz]

That'*s surely the best ... seems different preferences might exist 

Else i had not other findings. Wow! And it's so fast! (i'm using ownerdata for the shell listview)
 

[Bart]

I think I'll add an ExpandCollapseMode property:

Code: Pascal  [Select][+][-]

1. type
2.   TExpandCollapseMode = (
3.     ecmDefault,          // Current behaviour (--> default)
4.     ecmKeepExpanded,     // Do not clear children of already-expanded, but collaped nodes (d7-2-laz's code)
5.     ecmCollapseAndClear  // Clear children when node is collapsed
6.   );

ecmDefault is not very descriptive.
The fact that you make it default should already be reflected with the default modifier for the property.

Bart

[wp]

ecmLegacy? ecmClearExpandingAndKeepCollapsing?

[d7_2_laz]

ecmRefreshedExpanding?

[wp]

In the attachment there is a modified shellctrls.pas in which I attempt to implement the ExpandCollapseMode. Make a backup of your original shellctrls.pas in the lcl folder of your lazarus installation and copy the attached file over the original one.

The attachment contains also a small project for testing. I tried to measure the time it takes to expand or collapse a folder by storing the clock times at which the OnExpanding/OnExpanded or OnCollapsing/OnCollapsed event fire. My impression however is that this is not very correct since it feels much longer when the WinSxS folder collapses. However the qualitative difference between the ExpandCollapse modes is as expected

I also called GetHeapStatus.TotalAllocated to measure the change in occupied heap memory with respect to the memory used in the form's OnCreate event. Maybe there is some doubt on the exactness of these numbers, but again, the memory usage is as expected.

[d7_2_laz]

Thank you wp!
Seems it needs to be applied on a recent laz-main extract (due to other external dependencies), not the 2.2.4.

Tried the test project.
Of course i was focused onto the option 2 "Do not clear expanding nodes, keep collapsing nodes".
For the re-expand: yes, perfect! Just as expected.
For the collapse: when using option 2,  it appears to run same instantly too (if there should be any minimal delay, i think it's neglecticable).
Also, with option 2, a selected child node keeps to be selected when collapsing/re-expanding its parent node (no mystery, why); ok.

Exactly just how it should be ...   

[wp]

Committed the new shellctrls to main.

What's strange is that deletion of the child nodes in the C:\Windows\WinSxS subtree takes much longer than adding them...