[alpha] PLEASE TEST: Backgrounded MIME type loading, dir sizes, mount point refresh #582

Open
IgnorantGuru opened this Issue Sep 12, 2015 · 19 comments

Projects

None yet

6 participants

@IgnorantGuru
Owner

For those who would like to help with alpha testing, there is a temporary 'bg' alpha branch which includes some changes to dir loading and features. Because this branch made extensive changes to multi-threading behavior, it can use some heavy testing in assorted environments.

To install this branch, you can run the net installer:

spacefm-installer --version=alpha

Note that the alpha branch may not always exist (eg when no alpha-testing is underway). Automated build scripts should fall back to the 'next' branch if 'alpha' is missing.

This branch includes the following changes:

  • How SpaceFM loads a directory has changed. Previously, it loaded the directory like this:

    • Background Thread 1: Load the file list and file properties, and load the MIME types of all files (which involves opening/reading some files) and associated icons
    • UI Thread: Show the file list and let user perform functions.
    • Background Thread 2: Load thumbnails


    The above loading was slow when opening dirs like /usr/bin because the MIME type loading was slow.

    The new dir loading is performed like this:
    • Background Thread 1: Load the file list and file properties.
    • UI Thread: Show the file list and let user perform functions.
    • Background Thread 1: Load MIME Types and shows the icons.
    • UI Thread: When MIME Types are done loading, if sort is set to Type, the list will be resorted.
    • Background Thread 1: Calculate dir sizes
    • Background Thread 2 (started at same time as 'Calculate dir sizes' in another thread): Load thumbnails
    • UI Thread: When dir sizes are done loading, if sort is set to Size, the list will be resorted.

    Iow, the file list should be displayed almost immediately and you can begin using it as MIME types and icons are loaded, followed by dir sizes and thumbnails. (Issue #544)

  • Except for non-block filesystems listed in Devices|Settings|Change Detection, SpaceFM will calculate the deep size of subdirectories listed in the current directory. It will not list size for /proc, /mnt, or /sys. It will follow links in the current directory, but will not follow links within subdirectories. It will include the size of subdirectories mounted on another device, but will not includes files within the subdirectory which are mounted on another device. Thus it may differ from the size given by Properties|Info. It will roughly match the command 'du --apparent-size -hx'. (Thus if /media/cdrom is mounted, and the current directory is '/', the 'media' subdir will not include the size of /media/cdrom. Yet if you enter /media, then the size of 'cdrom' will be shown.) If subdirs cannot be read due to permissions, their size will not reflect contents. Dir sizes are not updated automatically - you must refresh or reopen the tab to get updated sizes. (Issue #338)

    <li>The View|Refresh function has been redesigned.  If multiple tabs are open to the same dir, refreshing one will now refresh all of them.  (Previously, SpaceFM was unable to reload the dir for refresh if multiple tabs were open to the same dir.)<br><br>
    
    <li>When mounting or unmounted a device, any tabs already open to the (canonicalized) mount point will be automatically refreshed.  They can also now be refreshed manually even if multiple tabs are open to the same mount point.  (Issue #568)<br><br>
    
    <li>A new run-task socket task type <a href="http://ignorantguru.github.io/spacefm/spacefm-manual-next-en.html#run-task-fresh">refresh</a> has been added to refresh tabs or dirs.<br><br>
    
    <li>Currently for testing, some additional output will be seen on stdout.  If SpaceFM crashes or hangs, please choose the Build Debug option in the installer, or see README.  Or just the steps if reproducible.
    

You can see recent changes to alpha branch, and also see all code changes between 'alpha' and 'next'.

If you have any questions or issues, please comment below. Also, feedback is welcome on the following:

Do you think there should be a global option to disable dir sizes? This functionality seems very useful and performs well, so thus far I haven't made it an option. If you think it should be optional, present your case and it will be considered (maybe try it for awhile first). Same for backgrounded MIME Type loading - I don't see any reason to make it optional.

Thanks for testing!

@IgnorantGuru IgnorantGuru added this to the near milestone Sep 12, 2015
@IgnorantGuru
Owner

A new run-task socket task type refresh has been added to refresh tabs or dirs.

@morfikov

I've tested this version, and now the issue #568 is gone completely. The content of a mount point appears in the open tab after mount the partition.

@IgnorantGuru IgnorantGuru added a commit that referenced this issue Sep 18, 2015
@IgnorantGuru [bg] dont request thumbnail thread unless old mtime #508; misc #582
added GDK_THREADS_ENTER around file listed signal in vfs-dir.c
removed GDK_THREADS_ENTER from notify_dir_refresh_idle in ptk-file-browser.c
28a9915
@IgnorantGuru IgnorantGuru added a commit that referenced this issue Sep 18, 2015
@IgnorantGuru [bg] vfs_dir_finalize remove dir hash before cancel task #582
otherwise a new tab loading/refreshing may try to ref this dir object while
the task is waiting for cancel
3b11065
@jmcamp
jmcamp commented Sep 25, 2015

One suggestion: since there's now a background thread to calculate directory sizes, can #245 be implemented within the same background thread with no extra performance penalty?

@IgnorantGuru
Owner

@jmcamp They're not really related. You're welcome to open a new request on that or add a comment to #17, which I would review before doing any work in that area, so it can be used to collect ideas for the dir tree.

@IgnorantGuru IgnorantGuru added a commit that referenced this issue Sep 26, 2015
@IgnorantGuru [bg] always cancel list thumbnail requests; Source ID not found debug #…
…582

This commit hangs on toggle show thumbnails

/* FIXME: Copying a large dir to current dir, then stopping the copy,
 * doing this repeatedly or when changing dir afterward (detailed
 * view), the thumbnail loader apparently runs on_thumbnail_idle
 * BEFORE g_idle_add_full, perhaps thread priority affecting printf output?
 * This condition is followed by a 'Source ID was not found' warning.
 * Using g_source_remove_by_user_data here instead causes a glib loop
 * thread deadlock on g_mutex_lock_slowpath (internal).  Even without it,
 * this deadlock still sometimes occurs.
 *
 * Abnormal debugging output show below:
 *
 * [normal]
 * vfs_thumbnail_loader_new 0x2e657c0 /tmp
 * g_idle_add_full@add1 0x2e657c0 2073
 * on_thumbnail_idle 0x2e657c0 2073
 * g_source_remove@on_thumbnail_idle 0x2e657c0 2073
 * vfs_thumbnail_loader_free 0x2e657c0
 *
 * [abnormal]
 * vfs_thumbnail_loader_new 0x5fe14c1014c0 /tmp
 * on_thumbnail_idle 0x5fe14c1014c0 0
 * g_idle_add_full@add1 0x5fe14c1014c0 2117
 * g_source_remove@vfs_thumbnail_loader_cancel_ 0x5fe14c1014c0 2117
 *
 * (spacefm:6284): GLib-CRITICAL **: Source ID 2117 was not found when attempting to remove it
 * vfs_thumbnail_loader_free 0x5fe14c1014c0
 */
9bb8694
@IgnorantGuru IgnorantGuru added a commit that referenced this issue Sep 26, 2015
@IgnorantGuru [bg] redesign vfs-async-task cancel/finalize; fix Source ID not found #…
…582

This uses a GCond to avoid hangs caused by depending on GDK_THREADS_LEAVE
6d40124
@IgnorantGuru IgnorantGuru modified the milestone: 1.0.4, near Sep 26, 2015
@IgnorantGuru IgnorantGuru modified the milestone: 1.0.5, 1.0.4 Oct 16, 2015
@IgnorantGuru
Owner

The above changes in the bg branch will not be included in the upcoming SpaceFM 1.0.4 release because of some remaining stability issues to be addressed. The bg branch has been merged into an 'alpha' branch, which can be used to continue alpha-testing this version.

I plan to use the 'alpha' branch from time to time for this kind of testing, rather than always using a different test branch name. However, the alpha branch may not always exist, so any build scripts should fall back to the 'next' branch if 'alpha' is missing.

This bg/alpha branch should be fine to use for most users - the instability will rarely if ever show itself through normal use. If it does, it may just cause a crash or hang. If you do encounter one, any details are welcome, particularly any warnings in stdout.

@IgnorantGuru IgnorantGuru changed the title from [bg] ALPHA TEST: Backgrounded MIME type loading, dir sizes, mount point refresh to [alpha] PLEASE TEST: Backgrounded MIME type loading, dir sizes, mount point refresh Oct 16, 2015
@paoloschi

Background Thread 1: Load the file list and file properties, and load the MIME types of all files (Which Involves opening / reading some files) and associated icons

here with 1.0.4+alpha if I list *.mod files in /boot/grub/i386-pc the description still says "Amiga SoundTracker audio"

expected behavior? or a misconfiguration of my system?

@IgnorantGuru
Owner

@paoloschi There haven't been any significant changes to how MIME types are determined, only when they're read in the dir loading process. *.mod files identify that way on my system too. This is done via glob in the file /usr/share/mime/audio/x-mod.xml. If you copy that file to ~/.local/share/mime/packages/audio-x-mod.xml and edit the comment (description), then restart SpaceFM, it should be changed for your current user in all apps. (You may also need to run update-mime-database as shown in the manual.)

Or, if you associate an application with that file type, then right-click on the application in SpaceFM's Open context menu, you can use the MIME Menu to copy that file, but you'll still need to edit it manually.

Identifying the file as an Amiga file by default is probably a bug or oversight in mime-shared-info library (Freedesktop), so that would be the upstream place to file a bug or request for change.

Thanks for testing.

@Vladimir-csp

Will the new threads system help reducing GUI hangs during long i/o wait?

@IgnorantGuru
Owner

@Vladimir-csp It can help reduce the time waiting for dir load, since more of it is now backgrounded (the MIME type identification requires opening some files, and this is now done in a background thread). So when I test with curlftpfs, for example, I notice the dir shows faster, then the MIME types are gradually loaded. So it seems more responsive.

However, aside from dir loading, stat() and other access calls will still hang. So clicking on a file in an unresponsive fs will still hang until the fs responds. In general, SpaceFM is as responsive as the filesystem being worked on. This change doesn't add asynchronous i/o for that GUI-related access.

Also note that dir size loading obeys the Devices|Settings|Change Detection blacklist.

@IgnorantGuru IgnorantGuru added a commit that referenced this issue Nov 8, 2015
@IgnorantGuru [bg] exo_icon_view_row_changed redraw item only #582
ptk-file-list.c on_thumbnail_loaded triggers exo_icon_view_row_changed which
previously redrew the entire layout. This commit only redraws a single item
unless item size changed.

This removes on_thumbnail_loaded from ptk-file-browser.c removing queue_draw
which was unreliable.  list->fast_update is now ignored.

FIXME: Sometimes on refresh thumbnails aren't shown in icon view until you
mouseover.  exo_icon_view_queue_draw_item gdk_window_invalidate_rect not
working properly?  Valid values seem to be passed to it.
6a3919d
@IgnorantGuru
Owner

Update: Problems with thumbnail loading (where a mouseover or scroll was required for the thumbnail to be updated, discussed elsewhere) should now be corrected.

Changed: A change has been made to the Change Detection blacklist in this branch, such that when a filesystem is blacklisted, newly created files will now be detected and read for their properties and MIME type. Deleted files have always been detected as well. Now only file content or property changes are ignored. This will make SpaceFM show files that are created, copied, renamed or deleted. Feedback is welcome on how this performs - is this desirable? The cost is that on less responsive filesystems, creating files may cause a hang until the new files are read.

@IgnorantGuru IgnorantGuru added a commit that referenced this issue Nov 9, 2015
@IgnorantGuru [bg] vfs-async-task g_thread_join replaces GCond method; remove locks #…
…582

The GCond method was deadlocking, perhaps due to rare races with
broadcast?

Removed unneeded mutex locks
8ec150a
@IgnorantGuru
Owner

I have made some improvements to the status bar - now shows total dir content size if no files are selected (this may differ from the dir size you see before navigating into the dir, because it only counts the files in the dir, not the dir itself), show derefed link sizes if single selection, and for multiple selection it shows the actual total (if you copied the selected files, this is how much data you're copying). This allows Icon users to click on a single dir or link to see the deep size. Comments welcome.

Also, if SpaceFM can't open a dir, no size is shown now, instead of 4.0K. (If files or dirs within the dir cannot be accessed, this is ignored with the total showing what can be read.)

Also, the device name (eg sdd1) holding the current dir is now shown at the left. Or for non-block filesystems, the fstype.

This branch is just about ready to go into next so any final testing or routine use is appreciated!

@Vladimir-csp

Works good so far. A little cosmetic issue on the status bar: IMHO, it is better to not show (0 B) at all when dir with unknown size is selected.

@IgnorantGuru
Owner

@Vladimir-csp

better to not show (0 B) at all when dir with unknown size is selected.

The size is not unknown per se. Rather it should show the accurate amount visible, just like if you run 'du -h /root'. But 0B is incorrect, should be something like 4.0K. I don't think it looks good to show nothing.

I have made a change where it now shows both the apparent size and the deep size when links are selected. And if a dir has zero size (no size shown in the list, indicating the size hasn't loaded yet or failed to open), then the lstat size is used (eg 4.0 K). Dirs like /sys (which aren't real) may still show 0 B, just like du does.

Long story short, the first amount shown is the amount that would be copied if you copied the selected files (roughly the same as du --apparent-size -hx DIR). The second number follow links (but not deep links). I think it's handy to be able to add up the defered dir links in some cases, so I think both numbers are useful, and seeing the arrow reminds you that some of what is selected is links.

Thanks for testing.

@IgnorantGuru
Owner

This alpha branch has been merged into next, due for 1.0.5 release. Any related problems can be noted below or opened in a new issue.

@IgnorantGuru IgnorantGuru changed the title from [alpha] PLEASE TEST: Backgrounded MIME type loading, dir sizes, mount point refresh to [next] PLEASE TEST: Backgrounded MIME type loading, dir sizes, mount point refresh Nov 10, 2015
@tome-
tome- commented Nov 12, 2015

I think there should be an option to disable dir sizes for example View -> Columns -> Size (files) and second Size (files and dirs). If you are opening dir, calculating and sorting dir sizes may take some time on slower devices so you can click something quickly but after a while it is something else.

@IgnorantGuru
Owner

If I add an option it would a global option in Preferences, as the dir size calc happens on the directory level which may impact multiple panels and windows. It would be like the Show Thumbnails option, perhaps putting it in View also so it can associated with keyboard and put on the toolbar.

Have you encountered any real-use difficulty with it, or are you hypothesizing? I tend to agree that some people may want the option, but I expected it to be more demanding than it is - it seems to work fluidly in the circumstances I've tested it.

@tome-
tome- commented Nov 13, 2015

It is real, I use 10 years old laptop and I hear my disk while it is reading size, something like short circuit. Before sorting by size, dirs are for a (too long) while sorted by name. I am hypothesizing only solutions, for example global option "Show dirs sizes" or button/key - "Calculte dirs sizes only for current tab/window".

@IgnorantGuru IgnorantGuru modified the milestone: 1.0.5, alpha Nov 25, 2015
@IgnorantGuru IgnorantGuru changed the title from [next] PLEASE TEST: Backgrounded MIME type loading, dir sizes, mount point refresh to [alpha] PLEASE TEST: Backgrounded MIME type loading, dir sizes, mount point refresh Nov 25, 2015
@IgnorantGuru
Owner

Update: Due to remaining instability related to these feature changes, these changes have been unmerged from the next branch and are once again available via only the alpha branch.

While the alpha branch should work fine most of the time, and I encourage use of this branch, it does not quite meet the quality control for release due to very rare crashes or hangs, eg #602.

While most of the issues causing instability have been resolved, there are still a few hard-to-find races that seem to be occurring between threads. Feedback on your results welcome, and I'll be continuing to use and bugfix this branch, getting it to release asap.

@IgnorantGuru IgnorantGuru added a commit that referenced this issue Nov 27, 2015
@IgnorantGuru new Pref option and builtin Style/toolbar item Show Folder Sizes #582
new socket property show_dirsize
04f9066
@IgnorantGuru
Owner

Update: A Show Folder Sizes option has been added to Preferences next to Show Thumbnails. This global option also appears in View|Style|Folder Sizes (where you can set a shortcut key), and you can add it as a new builtin toolbar item. There is also a new matching show_dirsize socket property.

When toggling this option, all open dirs will be refreshed. With the option disabled, no deep dir sizes should be calculated or shown.

@IgnorantGuru IgnorantGuru added a commit that referenced this issue Nov 27, 2015
@IgnorantGuru update lang ja #582 c6cd667
@IgnorantGuru IgnorantGuru added a commit that referenced this issue Jan 6, 2016
@IgnorantGuru [bg] Fix gdk_window warnings on refresh #582; maybe fixes #602
Fix in main-window.c:on_file_browser_after_chdir() fixes bug described in
ptk-file-browser.c:notify_dir_refresh() ?  grab_focus was cause?  Same
problem if placed directly in on_dir_file_listed().

This removes gdk_threads_enter in on_folder_view_item_sel_change_idle()
added in 6a3919d [alpha].  Not needed - may interfere?

Remove troublesome SEL_CHANGE signal emission from on_dir_file_listed() -
redundant due to on_folder_view_item_sel_change() called as idle. Caused
problems?
8d434ff
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment