ChannelHandler type hierarchy simplification #1999
Conversation
| * This method is not for a user but for the internal {@link ChannelHandlerContext} implementation. | ||
| * To trigger an event, use the methods in {@link ChannelHandlerContext} instead. | ||
| */ | ||
| void invokeWriteAndFlush(ChannelHandlerContext ctx, Object msg, ChannelPromise promise); |
There was a problem hiding this comment.
Not all handlers override both write() and flush(). Some will override only write() or only flush(). See the changes in DefaultChannelHandler.writeAndFlush(). What's your concern?
There was a problem hiding this comment.
No concern just was wondering why.
|
@trustin another thing... I'm not sure the @Deprecation of ChannelInboundHandler and ChannelOutboundHandler is useful as implementations of those will break. This is because ChannelHandler now has all the methods included and so implementations of ChannelInboundHandler/ChannelOutboundHandler will not be complete.. |
|
(Re: deprecation) I'll remove them. We should keep their adapters though, right? |
|
Yeah ... I think most users will use adapters anyway so most should not be affected hopefully. So after this and the handler name change please merge in
|
|
@trustin just did some benchmarks and it seems like it improves performance :) So once deprecation stuff is fixed please squash and merge in. |
|
@jpinner @wgallagher could you please review so we can move ahead ? |
- Move all methods in ChannelInboundHandler and ChannelOutboundHandler up to ChannelHandler - Remove ChannelInboundHandler and ChannelOutboundHandler - Deprecate ChannelInboundHandlerAdapter, ChannelOutboundHandlerAdapter, and ChannelDuplexHandler - Replace CombinedChannelDuplexHandler with ChannelHandlerAppender because it's not possible to combine two handlers into one easily now - Introduce 'Skip' annotation to pass events through efficiently - Remove all references to the deprecated types and update Javadoc
|
Merged via 110745b. |
ghost
assigned 
This changeset moves all methods in
ChannelInboundHandlerandChannelOutboundHandlerup toChannelHandlerand deprecate them and their adapters, greatly simplifying the type hierarchy of handlers by removing the distinction between 'inbound' handlers and 'outbound' handlers.Because all handlers are now both inbound and outbound handlers, it is not reasonable to provide
CombinedChannelDuplexHandler, whose purpose was to combine an inbound handler and an outbound handler into a single handler. Instead, I added a new handler calledChannelHandlerAppender, which appends the specified set of handlers together when it's added to the pipeline.It also introduces an annotation called
@Skipso thatDefaultChannelHandlerskips the evaluation of the handler methods annotated with@Skip. All handler methods inChannelHandlerAdapterare annotated with@Skip, so extendingChannelHandlerAdapterwill ensure that unoverridden methods are always annotated with@Skip. (i.e. most users don't need to know about this annotation unless they override a handler method and reverts back to pass-through mode.)Thanks to the
@Skipannotation, we can safely skip the evaluation of many unoverridden handler methods.Because the inspection of the presence of the annotation relies on reflection, I introduced a simple cache in
DefaultChannelHandlerContext. Is uses a partitioned synchronizedWeakHashMapso I'd say it has a room for improvements, but at the moment, I wouldn't do premature optimization.Talking about performance, I see slight boost, although it's not very noticeable. (0.5~1k HTTP QPS in my laptop) Simple distributed echo test on Mesos shows no regression (i.e. same latency/throughput. probably because it's just a single handler pipeline?)
This pull request is not complete yet - I'll clean all deprecation warnings and some rough edges soon.