Sync transfers (redux) for realistic progress bar updates

[vjr]

Fixes #2818

Alternative (simpler) approach to #2819

[vjr]

@jeremypw hmm i wonder if this might be a better (way better) approach to fix the issue? Thought to attempt this while working on #2827 to fix #137

[jeremypw]

Certainly simpler! But as far as I can see , when moving files your sync function is only called for local moves not for moves e.g. onto USB stick which is the opposite of what is needed?

I must admit it is hard to get one's head around all the C code 😞

As far as I can see move_files_prepare actually moves files in some cases (g_file_move succeeds without fallback flag) so the name is misleading. When this fails it calls copy_move_files and eventually g_file_move again with different flags allowing fallback. In the latter case the callback is still copy_file_progress_callback.

[jeremypw]

It would be nice to get the progress window to update in the middle of transferring a large file. At the moment it just freezes.

[vjr]

It would be nice to get the progress window to update in the middle of transferring a large file. At the moment it just freezes.

This appears to be the g_file_copy () invoking the callbacks very quickly likely due to the kernel buffer cache? So the "report copy move progress" function returns early because 100ms has not elapsed yet.

Should be addressed in f2a1017

It always reports the final callback/progress invocation and added a special case progress details text like "Please wait, finishing copy..."

[vjr]

Certainly simpler! But as far as I can see , when moving files your sync function is only called for local moves not for moves e.g. onto USB stick which is the opposite of what is needed?

I must admit it is hard to get one's head around all the C code 😞

As far as I can see move_files_prepare actually moves files in some cases (g_file_move succeeds without fallback flag) so the name is misleading. When this fails it calls copy_move_files and eventually g_file_move again with different flags allowing fallback. In the latter case the callback is still copy_file_progress_callback.

What are you doing that you do not see the syncs/progressbar updates happening for USB stick? Can you tell me the file size(s) and what steps you are doing? Seems to be working in whatever scenarios I've been testing.

[jeremypw]

@vjr The freezing mid-bar I previously noted does not seem to be happening now, thanks. I noticed the new "finishing moving ..." message when moving a large file onto the USB stick. Copying the same file onto the same stick was much slower when replacing an existing copy of the file and behaved differently in that the progress bar completed quickly (a few seconds) but then remained on "Please wait finishing copy ... " for a very long time even though the file transfer seemed complete as judged by the progress bar and the file size reported on the destination (after refresh). It eventually completed after 10 - 15 minutes. So the realistic progress updates do not seem to be working in that case.

[vjr]

@vjr The freezing mid-bar I previously noted does not seem to be happening now, thanks. I noticed the new "finishing moving ..." message when moving a large file onto the USB stick. Copying the same file onto the same stick was much slower when replacing an existing copy of the file and behaved differently in that the progress bar completed quickly (a few seconds) but then remained on "Please wait finishing copy ... " for a very long time even though the file transfer seemed complete as judged by the progress bar and the file size reported on the destination (after refresh). It eventually completed after 10 - 15 minutes. So the realistic progress updates do not seem to be working in that case.

@jeremypw not sure how this can be addressed. with my slow 8gb usb stick if i transfer (copy/move/overwrite) a file of say 6gb it previously appeared to finish quickly in some cases, but with this branch it will also take 10-15 minutes because the real transfer (write) speed of the usb stick is only 7-8 MBps so the duration is expected - previously it would give the user a false sense that the operation has safely compeleted.

[ProgramCrafter]

Should the code use fdatasync rather than fsync in the middle of the transfer, possibly?

[vjr]

Should the code use fdatasync rather than fsync in the middle of the transfer, possibly?

Hi yes thanks I did see the manpage earlier fdatasync seems a little better, fewer writes, especially reduced wear and tear on flash media, it excludes metadata I understand, until the final sync and close.

I guess we can just switch to it, do you have any additional insight into pros and cons beyond the docs?

[jeremypw]

@vjr I would try to resist the temptation to tinker with things not essential to the PR such as the wording of the progress dialog - it just clouds the issue. Just concentrate on fixing the linked issue(s). We can polish other features in subsequent PRs.

[jeremypw]

Could you add some comments to explain the difference (if any) between destination and destination_for_progress_dialog and their relation to target_dir in marlin_file_operations. Its best to add too many comments than not enough when the changes are not immediately obvious.

[vjr]

@vjr I would try to resist the temptation to tinker with things not essential to the PR such as the wording of the progress dialog - it just clouds the issue. Just concentrate on fixing the linked issue(s). We can polish other features in subsequent PRs.

OK agreed, I've reverted what I think are all the relevant commits you refer to in these new commits: 055dc3e, 3a26782 and e518d45.

[vjr]

Could you add some comments to explain the difference (if any) between destination and destination_for_progress_dialog and their relation to target_dir in marlin_file_operations. Its best to add too many comments than not enough when the changes are not immediately obvious.

@jeremypw comment added in 3a929f0 and also addressed your other feedback to avoid modifying the progess dialog texts.