search

Terry-Mao / goim

goim
https://goim.io/
MIT License
7.2k stars 1.78k forks source link

Proto在分发过程中被封装两次 #263

Closed yx2387 closed 5 years ago

yx2387 commented 5 years ago

新年好,

https://github.com/Terry-Mao/goim/blob/master/internal/job/room.go#L60-L95 在pushproc里, channel中的Proto被encode成byte array传到broadcastRoomRawBytes中

https://github.com/Terry-Mao/goim/blob/master/internal/job/push.go#L80-L88 在broadcastRoomRawBytes里, 又被放在另一个Proto的body里

请问这是一个bug吗? 谢谢!

tonybase commented 5 years ago

这里可以优化写法可能比较清晰，之前主要是考虑在job进行消息marshal好进行直接推送raw对comet不用再进行writeProto，其实对broadcast、room的消息效果会比较好一些，push user其实直接推送也可以。

Junxwan commented 5 years ago

依照上述的解释可以理解在以下程式码有做这件事 https://github.com/Terry-Mao/goim/blob/master/api/comet/grpc/protocol.go#L102 确实在job只做一次marshal，之后comet就不必在推送给每一个user时都做一次writeProto

那WriteWebsocket是不是也应该有这个逻辑？ https://github.com/Terry-Mao/goim/blob/master/api/comet/grpc/protocol.go#L185

是不是应该在

if buf, err = ws.Peek(_rawHeaderSize); err != nil {
      return
}

上面寫如下

if p.Op == OpRaw {
     err = ws.WriteBody(p.Body)
     return
}
Junxwan commented 5 years ago

后来看了examples/client.js发现我的理解有错，不用增加我上述的判断 因为job会做房间消息聚合，所以需要做WriteWebsocket逻辑让header可以说明body内总长度 再用for批次拿出每一条消息

tonybase commented 5 years ago

websocket这边确实需要多处理一次 batch body，而tcp那边倒是不用直接raw stream