Sync transfers for realistic progress bar updates

[vjr]

Fixes #2818

BEFORE:

files-progress-before.webm

AFTER:

files-progress-after.webm

[vjr]

I see the progress bar dialog is not "modal" so if I click the unmount button while the transfer is in progress Files crashes - but this is a different issue that already exists in the current main branch.

[jeremypw]

The value of 15 was set in 2011 by the original author (!) Certainly not appropriate for modern hardware especially SSD. I vaguely remember thinking that the linked issue was something to do with caching the write but if it fixes the issue for you there seems little harm in reducing this value. I'll check the code where this value is used though.

[jeremypw]

I see the progress bar dialog is not "modal" so if I click the unmount button while the transfer is in progress Files crashes - but this is a different issue that already exists in the current main branch.

The dialog is intended to be modal so we will have to block unmounting of any volume being written to (I am surprised the system does not do that) and show a warning if necessary. Doesn't the system show a "this volume is busy" warning?

[jeremypw]

I wonder whether this constant can be removed altogether or reduced to a small value 🤔 How fast can a reasonably accurate transfer rate be known? Maybe better to use a minimum number of bytes transferred rather than a fixed time?

[vjr]

I see the progress bar dialog is not "modal" so if I click the unmount button while the transfer is in progress Files crashes - but this is a different issue that already exists in the current main branch.

The dialog is intended to be modal so we will have to block unmounting of any volume being written to (I am surprised the system does not do that) and show a warning if necessary. Doesn't the system show a "this volume is busy" warning?

No warning, the app quits (crashes) if I click unmount while transfer is in progress. Should I open another PR to force the transfer dialog to be modal and not allow interacting with the app?

[vjr]

I wonder whether this constant can be removed altogether or reduced to a small value 🤔 How fast can a reasonably accurate transfer rate be known? Maybe better to use a minimum number of bytes transferred rather than a fixed time?

I've reduced the const down from 15 seconds to 3 seconds - seems reasonable for both slow and fast disks/hardware... if the hardware is fast the copy dialog completes and closes quickly anyway, if the hardware is slow then about 3 seconds seems okay for the transfer speed indication to show up?

[vjr]

The value of 15 was set in 2011 by the original author (!) Certainly not appropriate for modern hardware especially SSD. I vaguely remember thinking that the linked issue was something to do with caching the write but if it fixes the issue for you there seems little harm in reducing this value. I'll check the code where this value is used though.

Yep I thought as much haha. Thanks for looking at this PR. I think we can leave this reduced value in place for now, dont want to mess with further code changes to use a different way of calculating the duration before transfer speeds show up.

[flodavid]

Doesn't the system show a "this volume is busy" warning?

I have had the same issue. Another app failed to unmount one of my hard drive volume saying “This volume is busy”. So I tried with Files and it just crashed.

[jeremypw]

@flodavid Could you open an issue regarding your crash please? I'll try and look into file transfers again soon.

[vjr]

@flodavid Could you open an issue regarding your crash please? I'll try and look into file transfers again soon.

@jeremypw do you want me to make the transfer dialog modal thus preventing user interaction with the main app while it's visible? i can do that in this PR itself presuming it's a "one liner" change?

[jeremypw]

@Vishal No, I think how Files deals with trying to unmount a busy volume is better addressed with a separate issue/PR. File transfers should be non-modal and occur in the background if possible. Not sure how to stop people pulling out a USB stick prematurely We need to get a true (i.e. unaffected by caching) indication of whether a file transfer is ongoing/complete and show a warning and/or style the sidebar item approriately.

[vjr]

@Vishal No, I think how Files deals with trying to unmount a busy volume is better addressed with a separate issue/PR. File transfers should be non-modal and occur in the background if possible. Not sure how to stop people pulling out a USB stick prematurely We need to get a true (i.e. unaffected by caching) indication of whether a file transfer is ongoing/complete and show a warning and/or style the sidebar item approriately.

@jeremypw okay fair enough - so is this PR acceptable as an interim solution to make it more evident to users that a transfer is still in progress and to not unplug or unmount while so? further fixes/improvements in future PRs?

[jeremypw]

@vjr Yes. I wasn't able to test this today. Should do it tomorrow.

[jeremypw]

This appears to slow file transfers significantly. Maybe because the progress is more realistic? But it also seems to slow internal transfers between SSDs noticeably. I wonder if the syncing can be done less often and/or only when transferring to an external drive? Or should that wait for a future PR?

I noticed several areas that need cleaning up and reviewing in the file operations area (and it is still awaiting complete transfer to Vala) anyway.

I experimented with only syncing when a certain number of bytes had been transferred which seemed to reduce the impact on speed.

[vjr]

Yes I see the impact of the sync () calls slowing down transfers on fast media , it was not noticeable on the very slow media.

Plus, it might be that the global posix sync () call is not a good thing especially if there are multiple transfers in progress.

Marking this PR as draft while I come up with a better approach.

[jeremypw]

it was not noticeable on the very slow media.

I tested on a slow antique MTP player tranferring many small (100 byte) files and the reported transfer rate was about half with this PR. I must admit I did not manually time the complete transfer but cancelled before it finished. The situation is complicated by the fact that when transferring many small files the transfer slows down so the predicted time left is not very helpful - it often goes up instead of down. This was reported long ago but has never really been diagnosed. I think it may be to do with the "undo" processing taking longer and longer but I haven't dug into file operations nitty-gritty for a long time.

[vjr]

@jeremypw I've added a (WIP) custom copy_move_with_sync () intended to be a drop-in replacement for the g_file_copy () and g_file_move () glib/gio methods.

Yes, the periodic g_fsync () calls seem to be slowing down the transfers compared to the main branch so I'll boot up live sessions of ubuntu and kubuntu to compare against gnome and kde file managers, see what they do and how fast or slow they are.

I just tested individual file transfers of various sizes on two usb storage I have, a fast ssd and a slow flash stick, also tried multiple small files.

At the moment this new approach needs work to fix crashes/errors trying to copy directories, haven't yet tried other copy/move styles, folders, overwrite, recurse whatever.

Will continue working on this, see if the original goal of the issue (avoid users inadvertently unplugging their usb devices) can be resolved.

Thanks for looking into this.

[flodavid]

@jeremypw here it is: #2821. I assigned you to the issue.

[jeremypw]

@vjr Thanks for working on this! At the end of the day a (modest) reduction in speed is worth it to fix serious issues like crashes and data loss. I'll have another look at this tomorrow.