Improve folder enumeration speed #4199

gave92 · 2021-03-21T14:16:11Z

Resolved / Related Issues

Fixes #4028

Details of Changes

This PR:

~~Removes the caching feature which appears to slow down loading for small, local folders~~ -> restored
Temporary hidden files are not shown unless "show hidden items" is turned on (#4028)
Loads items faster when navigating back/forward
Hides the loading indicator as soon as the items from cache have been loaded -> @yaichenbaum could you test this and see if you like it?

yaira2 · 2021-03-21T14:26:03Z

@gave92 I'll test the changes.

gave92 · 2021-03-21T14:27:14Z

Thanks @yaichenbaum I'm curious to know if it's just my mind playing tricks xD

yaira2 · 2021-03-21T14:36:31Z

@gave92 I wouldn't say it's as fast as File Explorer, but it does seem to be faster than before.

gave92 · 2021-03-21T14:38:58Z

@yaichenbaum thanks, I feared the "maybe a bit faster" answer, probably going to close this xD

Edit: did you have any caching feature turned on before testing this PR?

yaira2 · 2021-03-21T14:42:50Z

@gave92 I thought this PR removes the cache option. Either way, it's a nice improvement and I think it's worth it to continue.

gave92 · 2021-03-21T14:44:17Z

@yaichenbaum Yep, funniest thing is that removing the caching feature makes things faster :) I'm going to leave this open to gather more feedback. If confirmed it would be a nice simplification of the code.

yaira2 · 2021-03-21T14:46:03Z

@gave92 It definitely makes things faster, and I can see items mush faster than before.

yaira2 · 2021-03-21T14:49:00Z

@jakoss Can you test these changes?

gave92 · 2021-03-21T15:39:23Z

@xpoppyx could you try again with the latest "Small fixes" commit?

gave92 · 2021-03-21T15:44:55Z

@xpoppyx thanks, I'll be interested in knowing how's loading speed for you (small/large/network folders) with this PR.

d2dyno1 · 2021-03-21T16:29:14Z

For #4180

gave92 · 2021-03-21T16:32:21Z

@xpoppyx I would not expect the issue to be tied to this PR, is this something that does not happen on main? Could you post the debug_fulltrust.txt?

gave92 · 2021-03-21T16:50:02Z

@xpoppyx my best guess is that you're getting this annoying issue -> #3163. If you find this happening on main and you manage to reproduce it consistently, I'm not above asking you to send me those folders/icons so I can reproduce the issue (and hopefully fix it) xD

gave92 · 2021-03-21T17:48:01Z

After testing in main, it is producible (with different steps) only after disabling cache.

Since this PR effectively disables the cache, I'm convinced the issue exists on main as well.

jakoss · 2021-03-22T10:28:54Z

It does seems to make the loading faster, the question here is at what point the cache became the choking point? Because all cache-related code shouldn't make that of a difference, even the database operations were at 50ms tops

gave92 · 2021-03-23T06:15:44Z

@yaichenbaum good for me. I'm gonna keep this to separate the other changes I ended up putting in here.

This reverts commit 7f4b83e.

… issue_4028

yaira2 · 2021-03-25T20:51:13Z

@gave92 It looks like scrolling is a little choppy when items are still loading. It also looks like it doesn't always work to change the layout mode. Overall, though, there seems to be a massive increase in performance. Do you think we should remove the cache setting and make it the default?

gave92 · 2021-03-25T21:30:10Z

Overall, though, there seems to be a massive increase in performance

Perceived increase, thing is currently for folders with less than 32 elements, it wastes time loading from the cache but never adds the items to the UI. With this PR cached items are shown also for small folders (plus there are a couple of tweaks that should actually reduce the loading time)

It looks like scrolling is a little choppy when items are still loading.

Yup, it shows cached items immediately but then it's busy loading the actual folder. Before you didn't notice because items appeared when it was done loading.

It also looks like it doesn't always work to change the layout mode

Could you clarify this?

Do you think we should remove the cache setting and make it the default?

~~No, personally I'd even turn caching off by default :) (or maybe keep in-memory cache only for file icons which helps when going back and forth)~~ maybe it's just the choppiness that puts me off.

Side note: Rx-Explorer has even faster loading performance and uses no cache whatsoever.

yaira2 · 2021-03-25T22:00:32Z

Could you clarify this?

Clicking on the different layout options in the display flyout don't always switch the layout mode.

Side note: Rx-Explorer has even faster loading performance and uses no cache whatsoever.

Is it faster than all the layout modes, or just the one with the DataGrid?

gave92 · 2021-03-25T22:08:29Z

@yaichenbaum mostly tried in details view only (they use a ListView underneath I think, not DataGrid which may explain some of the difference?)

yaira2 · 2021-03-25T22:12:55Z

@yaichenbaum mostly tried in details view only (they use a ListView underneath I think, not DataGrid which may explain some of the difference?)

That's why I asked, I figured the DataGrid might be part of the slowness. Regarding the cache, if we can improve the speed, we can definitely remove the cache altogether.

winston-de · 2021-03-26T04:22:05Z

I noticed this PR significantly decreases the loading speed of shortcuts (I'm assuming because now they have to be obtained through the FTP every time). This may not be feasible, but perhaps just shortcuts could be cached since they take a lot longer to load than other items?

gave92 · 2021-03-26T06:06:24Z

@winston-de at the end we did not remove the caching feature so it shouldn't take longer to load folder contents. Do you find that a folder with a lot of shortcuts now loads slower?

winston-de · 2021-03-26T07:03:54Z

@gave92 It looks like the json deserializer was breaking when trying to deserialize shortcut items, which slowed down the loading speed. Oddly enough, it seems to have stopped, and the loading speed is just as fast as other folders.

Here was the error that was causing it:

2021-03-25 21:17:46.7340|WARN|.ctor|Unable to find a constructor to use for type Files.Filesystem.ShortcutItem. A class should either have a default constructor, one constructor with arguments or a constructor marked with the JsonConstructor attribute. Path 'FileList[0].TargetPath', line 1, position 74.|Newtonsoft.Json.JsonSerializationException: Unable to find a constructor to use for type Files.Filesystem.ShortcutItem. A class should either have a default constructor, one constructor with arguments or a constructor marked with the JsonConstructor attribute. Path 'FileList[0].TargetPath', line 1, position 74.
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateNewObject(JsonReader, JsonObjectContract, JsonProperty, JsonProperty, String, Boolean&) + 0x17d
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObject(JsonReader, Type, JsonContract, JsonProperty, JsonContainerContract, JsonProperty, Object) + 0x2f8
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader, Type, JsonContract, JsonProperty, JsonContainerContract, JsonProperty, Object) + 0xb6
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.PopulateList(IList, JsonReader, JsonArrayContract, JsonProperty, String) + 0x3d5
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateList(JsonReader, Type, JsonContract, JsonProperty, Object, String) + 0x110
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader, Type, JsonContract, JsonProperty, JsonContainerContract, JsonProperty, Object) + 0xef
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.SetPropertyValue(JsonProperty, JsonConverter, JsonContainerContract, JsonProperty, JsonReader, Object) + 0x16d
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.PopulateObject(Object, JsonReader, JsonObjectContract, JsonProperty, String) + 0x6b2
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObject(JsonReader, Type, JsonContract, JsonProperty, JsonContainerContract, JsonProperty, Object) + 0x321
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader, Type, JsonContract, JsonProperty, JsonContainerContract, JsonProperty, Object) + 0xb6
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.Deserialize(JsonReader, Type, Boolean) + 0x22d
   at Newtonsoft.Json.JsonSerializer.DeserializeInternal(JsonReader, Type) + 0xf3
   at Newtonsoft.Json.JsonConvert.DeserializeObject(String, Type, JsonSerializerSettings) + 0xd6
   at Newtonsoft.Json.JsonConvert.DeserializeObject[T](String, JsonSerializerSettings) + 0x36
   at Files.Helpers.FileListCache.PersistentSQLiteCacheAdapter.<ReadFileListFromCache>d__3.MoveNext() + 0x2e6

However, this is not an issue with this pr, as my log shows the same error going back to at least February. I'll file a new issue for this.

gave92 · 2021-03-26T07:05:41Z

#4283 is supposed to have fixed that issue (adds empty constructor for shortcut listed item)

winston-de · 2021-03-26T07:15:58Z

Hmmmm I'm still getting the error. I'll try clearing the cache, maybe that'll fix it.

gave92 added 4 commits March 21, 2021 14:26

Fix deprecation warnings in ItemViewModel

82cae48

Removed extra call to ApplyFilesAndFoldersChangesAsync

381e5c8

Do not show temporary files when hidden items are not shown

6e1f996

Remvoe caching feature

7f4b83e

gave92 changed the title ~~Issue 4028~~ Improve folder enumeration speed Mar 21, 2021

gave92 added the help wanted Extra attention is needed label Mar 21, 2021

gave92 removed the help wanted Extra attention is needed label Mar 21, 2021

gave92 marked this pull request as ready for review March 21, 2021 14:46

Merge remote-tracking branch 'origin/main' into issue_4028

1fcff23

yaira2 added the needs - code review label Mar 21, 2021

yaira2 previously approved these changes Mar 21, 2021

View reviewed changes

Small fixes

273da40

gave92 dismissed yaira2’s stale review via 273da40 March 21, 2021 15:35

Remove extra call to CheckCloudDriveSyncStatusAsync

a70e415

gave92 mentioned this pull request Mar 21, 2021

[Testing only] - Load icons from FTP only for hidden and shortcut files #4205

Closed

2 tasks

gave92 added 7 commits March 24, 2021 21:16

Revert "Remvoe caching feature"

640070a

This reverts commit 7f4b83e.

Fix wrong thread warning in log file && fix merge conflicts

4c2b8c3

Merge branch 'issue_4028' of https://github.com/gave92/files-uwp into…

b9f74ff

… issue_4028

Revert xlf changes

06baf93

Revert ewsw changes

75ead99

Small fixes

5396c1d

Merge remote-tracking branch 'origin/main' into issue_4028

c5a2a7a

gave92 changed the title ~~Remove caching feature~~ Improve folder enumeration speed Mar 24, 2021

gave92 added 2 commits March 24, 2021 22:21

Always save at most 32 items to cache

6a471e7

Tweak cache loading

3eebae7

gave92 force-pushed the issue_4028 branch from 788dc62 to 3eebae7 Compare March 24, 2021 23:29

gave92 marked this pull request as ready for review March 24, 2021 23:40

gave92 added 2 commits March 25, 2021 21:00

Merge remote-tracking branch 'origin/main' into issue_4028

066efd2

Show empty icon when loaded from disk cache

e4c9bf5

yaira2 approved these changes Mar 25, 2021

View reviewed changes

yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Mar 25, 2021

yaira2 merged commit 89a743c into files-community:main Mar 25, 2021

gave92 deleted the issue_4028 branch March 25, 2021 22:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve folder enumeration speed #4199

Improve folder enumeration speed #4199

gave92 commented Mar 21, 2021 •

edited

Loading

yaira2 commented Mar 21, 2021

gave92 commented Mar 21, 2021

yaira2 commented Mar 21, 2021

gave92 commented Mar 21, 2021 •

edited

Loading

yaira2 commented Mar 21, 2021

gave92 commented Mar 21, 2021

yaira2 commented Mar 21, 2021

yaira2 commented Mar 21, 2021 •

edited

Loading

gave92 commented Mar 21, 2021

gave92 commented Mar 21, 2021

d2dyno1 commented Mar 21, 2021

gave92 commented Mar 21, 2021

gave92 commented Mar 21, 2021

gave92 commented Mar 21, 2021

jakoss commented Mar 22, 2021

gave92 commented Mar 23, 2021 •

edited

Loading

yaira2 commented Mar 25, 2021 •

edited

Loading

gave92 commented Mar 25, 2021 •

edited

Loading

yaira2 commented Mar 25, 2021 •

edited

Loading

gave92 commented Mar 25, 2021

yaira2 commented Mar 25, 2021

winston-de commented Mar 26, 2021 •

edited

Loading

gave92 commented Mar 26, 2021 •

edited

Loading

winston-de commented Mar 26, 2021 •

edited

Loading

gave92 commented Mar 26, 2021 •

edited

Loading

winston-de commented Mar 26, 2021

Improve folder enumeration speed #4199

Improve folder enumeration speed #4199

Conversation

gave92 commented Mar 21, 2021 • edited Loading

yaira2 commented Mar 21, 2021

gave92 commented Mar 21, 2021

yaira2 commented Mar 21, 2021

gave92 commented Mar 21, 2021 • edited Loading

yaira2 commented Mar 21, 2021

gave92 commented Mar 21, 2021

yaira2 commented Mar 21, 2021

yaira2 commented Mar 21, 2021 • edited Loading

gave92 commented Mar 21, 2021

gave92 commented Mar 21, 2021

d2dyno1 commented Mar 21, 2021

gave92 commented Mar 21, 2021

gave92 commented Mar 21, 2021

gave92 commented Mar 21, 2021

jakoss commented Mar 22, 2021

gave92 commented Mar 23, 2021 • edited Loading

yaira2 commented Mar 25, 2021 • edited Loading

gave92 commented Mar 25, 2021 • edited Loading

yaira2 commented Mar 25, 2021 • edited Loading

gave92 commented Mar 25, 2021

yaira2 commented Mar 25, 2021

winston-de commented Mar 26, 2021 • edited Loading

gave92 commented Mar 26, 2021 • edited Loading

winston-de commented Mar 26, 2021 • edited Loading

gave92 commented Mar 26, 2021 • edited Loading

winston-de commented Mar 26, 2021

gave92 commented Mar 21, 2021 •

edited

Loading

gave92 commented Mar 21, 2021 •

edited

Loading

yaira2 commented Mar 21, 2021 •

edited

Loading

gave92 commented Mar 23, 2021 •

edited

Loading

yaira2 commented Mar 25, 2021 •

edited

Loading

gave92 commented Mar 25, 2021 •

edited

Loading

yaira2 commented Mar 25, 2021 •

edited

Loading

winston-de commented Mar 26, 2021 •

edited

Loading

gave92 commented Mar 26, 2021 •

edited

Loading

winston-de commented Mar 26, 2021 •

edited

Loading

gave92 commented Mar 26, 2021 •

edited

Loading