-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
feat: 优化缓冲发送逻辑,减少缓冲发送小数据包的频繁系统调用 #358
Conversation
znet/connection.go
Outdated
if c.isClosed() == true { | ||
return errors.New("connection closed when send msg") | ||
} | ||
c.bufMu.Lock() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
只会有一个线程往缓冲区写入,写到 bufio
应该不需要锁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我怕的是同时发生send和sendbuf两个方法调用,然后sendbuf刚好满了,触发了tcp的Write,会不会造成底层连接的脏写?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
还有就是bufio的flush时和bufio的write时,bufio底层没有锁控,看gpt的说法是最好加个锁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我怕的是同时发生send和sendbuf两个方法调用,然后sendbuf刚好满了,触发了tcp的Write,会不会造成底层连接的脏写?
可以在 sendbuf()
的时候调用一下 Available()
看一下容量如果写入的 size 大于容量可以手动调用一次 flush()
这里会等 send()
的锁结束,然后 bufio
确实底层没有锁,但是 flush()
和 witre()
目前写法应该不会一起发生的,在一个 for{ select{} }
里,不过加锁也没大问题,只是我想尽量不要影响立即发送的 send()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
好像确实在一个for select中不会发生这种问题,但是在发送前调用available手动flush感觉有点在重复干bufio.write()的活,我再看看,尽量减少锁的使用吧
@aceld |
@YanHeDoki 有点记不住了,不过我看了下PR的代码,这里是在io层统一出口处,加了buffer,目的是放置多次走send。理论上应该问题不大,我看那个锁的部分也去掉了。 |
@EquentR 感谢提交PR! |
感谢您的认可!😄 |
fix: #357