shadowsocks / shadowsocks-android

A shadowsocks client for Android
Other
35.25k stars 11.57k forks source link

Add support for "optimize tcp buffers" option per profile. #3137

Open notsure2 opened 11 months ago

notsure2 commented 11 months ago
Mygod commented 2 months ago

Hi please see here for the translation contribution: https://discourse.shadowsocks.org/t/poeditor-translation-main-thread/30

Is there a reason to still use the default buffer size? Why not enforce this size?

notsure2 commented 2 months ago

@Mygod Thank you for looking at this PR.

The small buffer size should be only incoming socket if plugin is not used, and in case of plugin used both incoming and outgoing socket to plugin should have their buffers stripped down.

I'm not sure what to do about translation ? I should remove from PR and add in that POEditor website ?

Mygod commented 2 months ago

Only include the English texts and add them later.

Thanks for the PR but I think if the issues are real then shadowsocks-rust should probably provide a better default instead. @zonyitoo Thoughts?

notsure2 commented 2 months ago

I kind of disagree it will be a breaking change of behavior to all users who are not aware the effect of this change, that's why I don't even default this setting to on. The goal of this change is fix upload buffer bloat by removing buffer between app and ss and between ss and cloak in upload direction.

notsure2 commented 2 months ago

@Mygod done

Mygod commented 2 months ago

So is this a Cloak specific issue? Other plugins don't seem to have this problem.

notsure2 commented 2 months ago

So is this a Cloak specific issue? Other plugins don't seem to have this problem.

Negative, this affects all plugins actually. It's most visible when using a connection with a slow upload. It looks like this, try to upload a large file with ss or ss + plugin without optimize buffers, on a slow upload internet.

What you will see, upload will jump to a high % immediately, then slowly actually upload to the background: Cause is that the browser copied to ss buffer very quickly and ss accepted it because default buffering is huge, but actually the underlying upload can't handle it, so the upload just hangs a for a long time with no progress until it catches up, then continues at the real speed to the end.

Many upload controls will cancel the upload with "Timeout" error if no bytes move for a long time (which happens in this case due to the buffer bloat at the local ss).

App ----------------> ss ------------------> internet
        (balloon here)         

In ss alone the balloon must be small or it will inflate with data much faster than ss can empty it to the internet giving a false impression of upload speed to the app potentially leading to timeouts as the app thinks the connection stalled.

App ----------------> ss ------------------> plugin --------------------> internet
        (balloon here)         (balloon here)

Same problem here, the idea is all buffers from the app to the internet except the last one must be set very small so that the app gets a correct TCP backpressure and correct "feeling" of upload speed and works properly.

PS: This problem doesn't affect download, only upload

Mygod commented 2 months ago

It seems that the default buffer size is 16383*20 bytes unless I'm mistaken? This probably wouldn't cause the issue you described unless I'm mistaken.

notsure2 commented 2 months ago

That's 20 times what I set :-) I set to just 16384. Also default is whatever is core_wmem i think in kernel, for localhost it's very big.