Fix path used in sabre's tree cache

[icewind1991]

Sabre's getChildren() cached path with a leading slash, now we properly used entries cached by getChildren() which prevents us from having to re-query the fileinfo.

Greatly improves performance when doing a Depth=1 PROPFIND (comparison)

cc @DeepDiver1975 @PVince81 @LukasReschke

[DeepDiver1975]

is there unit test coverage on the tree?

[icewind1991]

Yes

[rullzer]

👎

Let me explain. What happens is that the cache is used and filled in the function getNodeForPath. But it is also filled in the function getChildren https://github.com/owncloud/3rdparty/blob/master/sabre/dav/lib/DAV/Tree.php#L192-L204

The trouble is at https://github.com/owncloud/3rdparty/blob/master/sabre/dav/lib/DAV/Tree.php#L196 that for child nodes puts a '/' between the basePath and the childs name. Now of course when you are in the root there is no basePath so the child nodes get cached like /<CHILD>. However if you a subfolder they get cached like <SUBFOLDER>/<CHILD>.

So your patch will speedup PROPFINDS in the root but slow down in all subfolders.

Basically this is a bug in sabre. Or at least in how we use it ;) We could fix it by copyng the code of getChildren over an fixing it. But probably better to fix upstream. @evert do you want me to prepare a PR against 2.1.7 or will there be no 2.1.8?

[rullzer]

Upstream PR in https://github.com/fruux/sabre-dav/pull/729

[rullzer]

Of course I did some measurements:

propfind on root with 5 folders and 5 files

what	time	graph
master	306ms	graph
master + this patch	70.7ms	graph
master + sabre patch	47.3ms	graph

propfind on subfolder with 5 folders and 5 files

what	time	graph
master	72.9ms	graph
maser + this patch	223ms	graph
master + sabre patch	69.6ms	graph

[LukasReschke]

Awesome work @icewind1991 and @rullzer 🚀

[PVince81]

I suggest to provide a method "setCachedEntry" that auto-adds or auto-strips the leading slash. It's not the first time this kind of thing happens, had this lately with Dropbox local cache too: see #19877

[rullzer]

Fix is in sabre master. So we have yet another reason to upgrade ;)
Let close this then.

[DeepDiver1975]

Fix is in sabre master. So we have yet another reason to upgrade ;)
Let close this then.

in order to get this fixed in 8.2 and earlier we need a fix in 2.1 - requested at fruux/sabre-dav@6223d06#commitcomment-14005068

[ghost]

@rullzer @DeepDiver1975 @icewind1991 Just stumbled over this. 2.1.9 (#21914) has included this fix.