[GitHub] sijie commented on issue #1286: Implement directly ChannelOutboundHandlerAdapter in BookieProtoEncoding#ResponseEncoder

[GitBox <gi...@apache.org>]

sijie commented on issue #1286: Implement directly ChannelOutboundHandlerAdapter in BookieProtoEncoding#ResponseEncoder
URL: https://github.com/apache/bookkeeper/pull/1286#issuecomment-375759930
 
 
   > MessageToMessageEncoder uses reflection and does checks on objects types and it does other things not needed so my idea is that we are saving CPU cycles.
   
   "reflection" is only used during initialization on constructing the type matcher, no?
   
   "checks on objects" -> the type we are using is Object and it is calling isInstance, which it is a very cheap op.
   
   " my idea is that we are saving CPU cycles." -> I am not sure we are actually saving anything here.
   
   --------------
   
   To be honest, from code perspective, personally I would prefer MessageToMessage{Encoder,Decoder} because we don't need to handle "refcount release" stuff, which can become very tricky with netty version upgrades (e.g. if netty changes some of the behaviors)
   
   However I understand you want to address your production issue and it seems this change can address your issue. I am fine with change as if the description is updated to what is actually changed here.
   
   
   

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


With regards,
Apache Git Services