Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: non-blocking ChannelTransferProgress handling #36

Conversation

black-night-heron
Copy link
Contributor

@black-night-heron black-night-heron commented Apr 20, 2024

Fixes #32.

In the current implementation, time.Sleep(50 * time.Millisecond) is called whenever a ChannelTransferProgress event is triggered, and the channel handling these events is unbuffered. This setup causes the reader/writer processes to be blocked, subsequently slowing down both upload and download speeds.

This pull request proposes a solution by leveraging the built-in throttle feature of the progressBar instead of using time.Sleep.


Possibility of a Non-blocking Send

Actually, integrating the progressBar's throttle feature introduces overhead due to various checks and locks. An alternative improvement could be the implementation of non-blocking sends, which might further enhance performance:

--- a/sdk/client.go
+++ b/sdk/client.go
@@ -204,7 +204,11 @@ func (client *Client) signalTransferProgress(b int64) {
        if !client.UseTransferSignals {
                return
        }
-       client.ChannelTransferProgress <- b
+
+       select {
+       case client.ChannelTransferProgress <- b:
+       default:
+       }
 }

However, this change in the SDK could potentially lead to some issues and may break other SDK users. For instance, the last progress update might be dropped, which could result in an incomplete progress bar display:

Uploading 100MB.bin...          99% |███████████████████ | (105/105 MB, 5.1 MB/s) [32s:0s]

@black-night-heron black-night-heron force-pushed the fix/non-blocking-receiving-progress branch from 085e80d to 8a92fc6 Compare April 20, 2024 10:38
@virtualzone virtualzone merged commit 8c7d6bf into virtualzone:main Apr 25, 2024
1 of 2 checks passed
@virtualzone
Copy link
Owner

Thanks for your contribution @black-night-heron !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upload speed much lower than via the browser
2 participants