Page MenuHome

Fix T69530: Remove Lag in File Browser
ClosedPublic

Authored by Harley Acheson (harley) on Oct 21 2019, 8:04 PM.

Details

Summary

This fix might look funny as it is just removing everything from space_file init().

But the things in the spaceType init() callback are not called just during initialization, but is called all over, multiple times in a row, and all the time constantly while doing some things.

Every time you open a File Browser the following is done:

  • Emptying and Rebuilding Volumes and System Lists
  • Stop Job that validates bookmarks
  • Start Job that validates bookmarks
  • Emptying and Rebuilding Volumes and System Lists
  • Stop Job that validates bookmarks
  • Start Job that validates bookmarks
  • Emptying and Rebuilding Volumes and System Lists
  • Stop Job that validates bookmarks
  • Start Job that validates bookmarks
  • Emptying and Rebuilding Volumes and System Lists
  • Stop Job that validates bookmarks
  • Start Job that validates bookmarks

And then while just dragging the File Browser window around you get constant calls to do:

  • Emptying and Rebuilding Volumes and System Lists
  • Stop Job that validates bookmarks
  • Start Job that validates bookmarks
  • Emptying and Rebuilding Volumes and System Lists
  • Stop Job that validates bookmarks
  • Start Job that validates bookmarks
  • ....

None of these are necessary. The Volumes and Systems lists are initially populated elsewhere. And if you truly need to reload the lists and re-validate the items in the user bookmarks, these things are done when clicking the "refresh" button.

Diff Detail

Repository
rB Blender

Event Timeline

Generally think that is OK, refreshing those system bookmarks indeed do not need to be done that often, especially since that can hang pretty badly on some systems/FS combinations...

source/blender/editors/space_file/space_file.c
187–189

Am not sure about that removal... Or is this also taken care of elsewhere?

@Bastien Montagne (mont29) - Generally think that is OK...Am not sure about that removal... Or is this also taken care of elsewhere?

Sort of. LOL. That's the problem.

When these things are in the "Init" they are called often and all the time for no reason. But we don't (seem to) have somewhere else they can be called just a bit less.

So with "Init" emptied, as I propose, the list of Volumes, System, Favorites, Bookmarks are all initially filled quite nicely earlier. Things work perfectly, but just better without the background threads starting and stopping and recreating those lists and verifying the paths constantly, etc.

However, they are also not being refreshed automatically. So if you make an invalid drive valid (by putting in media perhaps), or add a new network drive, then you'd have to press the "Refresh" button, shown on the header. That seems reasonable to me. But might not be reasonable to anyone else.

Not updating system bookmarks automatically is fine-ish, we do not update the file browser listing itself either when e.g. a new file is created in the current directory or so...

My concern is about the dirty flag of the layout (check the lines related to the comment), this is not only affecting bookmarks I believe, but also whole main area drawing? Hence my question...

@Bastien Montagne (mont29) My concern is about the dirty flag of the layout (check the lines related to the comment), this is not only affecting bookmarks I believe, but also whole main area drawing? Hence my question...

Oh, sorry for the confusion.

AFAICT the only reason the layout is set dirty at that point is so that the area will redraw because of the (potential) changes to those lists. But if those two things are removed then there is no change that happens at that point and no reason to set dirty. The layout is properly set dirty in other places where changes occur or redrawing must happen. So with this area cleared it still redraws at the right times, when moving and resizing, etc

OK, then think that is OK. Easy to fix in any case should unexpected issues arise.

This revision is now accepted and ready to land.Tue, Nov 26, 6:43 PM

As raised by @Bastien Montagne (mont29) there is a situation where it is needed to have the FileLayout set dirty in init(). When resizing the area while embedded, like in Shading workspace.

Patch updated to leave that part in, and only remove the calls to refresh the fsMenu lists.

In case you are considering reverting this patch, some notes...

Although I have found no ill effects from this change, you might be here because of something I did not notice. Best to read the following rather than revert as the old behavior was far from ideal.

1. The initial list is somehow incomplete.

Currently the initial list is built in wm_init_exit.c at about line 264 with a call to ED_file_init(). If you need it to happen later, like after reading user prefs for example, then move that call to line 307 or so.

2. You need to refresh the list more often.

If you need the lists to be refreshed more often, there is a place to do so. The old code did so four times as the browser was opening and far too many times after. But if you want it to happen just ONCE when browser is opened (and just once when going to a workspace that include file browser), then add "ED_file_init()" in space_file.c file_listener() at about 356. So category NC_SPACE, data ND_SPACE_FILE_LIST, just before the "ED_area_tag_refresh(sa)"