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

mux cool: Add maxReuseTimes #4231

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

mux cool: Add maxReuseTimes #4231

wants to merge 2 commits into from

Conversation

Fangliding
Copy link
Member

@Fangliding Fangliding commented Dec 31, 2024

字面意思
现 mux cool 逻辑:
MaxConcurrency 为一个连接里的最大活动子连接数
还有一个神秘的 MaxConnections 参数 硬编码为128 实际上就是最大复用次数 我不知道为什么叫这个 这个PR将其重命名了
我修改了一下 允许手动指定 maxReuseTimes 并把默认值变成了65535(mux.cool两位流ID允许的最大上限)

这些限制仅存在于客户端 服务端没有限制 旧的可以正常工作

@RPRX
Copy link
Member

RPRX commented Dec 31, 2024

还有一个神秘的 MaxConnections 参数 硬编码为128 实际上就是最大复用次数 我不知道为什么叫这个 这个PR将其重命名了

之前我也发现了这个,我觉得现在要加参数的话不如加上与 XMUX 等价的所有参数,所以明年再合

@yuhan6665
Copy link
Member

现在要加参数的话不如加上与 XMUX 等价的所有参数

这个我支持R佬
长远来看 XMUX 应该对 mux-cool 以及所有有 mux 效果的 transport 起作用

@Fangliding
Copy link
Member Author

我也不反对 不过这都是后话了 其他部分还有包括让它支持range什么的都需要一定的修改 而且就连xmux自己刚刚都还有一些参数修改 这里只是把这个已经有的逻辑暴露出来

@yuhan6665
Copy link
Member

所以我一开始就说配置放外面
#3613 (comment)

@Fangliding
Copy link
Member Author

image
之前甚至还想过要不要轮转重用前面的子连接ID 65535可能有点短 看起来应该不用 除了被劣质tdesktop一秒钟刷几个post一般到个几千万把到头了

@alphax-hue3682
Copy link

alphax-hue3682 commented Jan 1, 2025

还有一个神秘的 MaxConnections 参数 硬编码为128 实际上就是最大复用次数 我不知道为什么叫这个 这个PR将其重命名了

之前我也发现了这个,我觉得现在要加参数的话不如加上与 XMUX 等价的所有参数,所以明年再合

I think you should try to integrate this pr and then it will be completed in the rest of the version, But still, your opinion is preferable

@RPRX
Copy link
Member

RPRX commented Jan 1, 2025

@alphax-hue3682 I'm not intend to introduce unstable changes between v24.12.31 and v25.1.1. The diffs should be tiny.

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.

4 participants