zhijiangW commented on issue #7368: [FLINK-10742][network] Let Netty use 
Flink's buffers directly in credit-based mode
URL: https://github.com/apache/flink/pull/7368#issuecomment-582745768
 
 
   I get the general idea of the implementation, but have not thought the 
details through yet. This improvement only validates for the body of 
`BufferResponse` message on receiver side, and we still have memory overheads 
for other messages and the frame and header parts of `BufferResponse`.
   
   I am not caring about the other messages because they are less frequent in 
running except `AddCredit` which might have the same frequency with 
`BufferResponse`. Although the size of `AddCredit` is very small, I am not sure 
whether it would still boost the netty memory overhead in practice.
   
   I noticed that the sender side also reuses the same new 
`ZeroCopyNettyMessageDecoder` with receiver side, and I have some concerns 
here. This new decoder would maintain/manage the overhead memory for frame and 
header parts compared with previous embedded `LengthFieldBasedFrameDecoder` in 
netty. If the new decoder is more efficient or memory saving than the 
`LengthFieldBasedFrameDecoder` (e.g. for the case of `AddCredit` mentioned 
above), it is reasonable, otherwise I do not prefer to reuse it on sender side 
for two reasons:
   
   - To do so, actually we do not unify the decoder on both sender and receiver 
side, because we also define different `NettyMessageParser` implementations 
inside `ZeroCopyNettyMessageDecoder`, which have the same effect with different 
decoders on sender and receiver sides.
   
   - It brings more changes and seems more complex. We can retain the previous 
`NettyMessageDecoder` for sender side and only removes the `BufferResponse` 
path from it, then we define a different decoder only for receiver side.
   
   Next I would further think how to refactor the implementation of 
`ZeroCopyNettyMessageDecoder`.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to