Skip to content
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

Type Pollution Scalability issue over Vertx/Netty HTTP 2 pipeline traversal #5039

Open
franz1981 opened this issue Dec 19, 2023 · 2 comments · Fixed by #5092
Open

Type Pollution Scalability issue over Vertx/Netty HTTP 2 pipeline traversal #5039

franz1981 opened this issue Dec 19, 2023 · 2 comments · Fixed by #5092
Labels

Comments

@franz1981
Copy link
Contributor

franz1981 commented Dec 19, 2023

As reported (and fixed) at netty/netty#12806, only ChannelDuplexHandlers in netty have been fixed to solve the infamous type pollution issue: by running an hello-world like test with HTTP 2 in quarkus and attaching https://github.com/RedHatPerf/type-pollution-agent to it, this case popup

2:      io.vertx.core.http.impl.VertxHttp2ConnectionHandler
Count:  32816944
Types:
        io.netty.handler.codec.http2.Http2Connection$Listener
        io.netty.channel.ChannelInboundHandler
        io.netty.channel.ChannelOutboundHandler
        io.netty.handler.codec.http2.Http2LifecycleManager
Traces:
        io.netty.handler.codec.http2.DefaultHttp2Connection$DefaultEndpoint.addStream(DefaultHttp2Connection.java:805)
                class: io.netty.handler.codec.http2.Http2Connection$Listener
                count: 7912545
        io.netty.handler.codec.http2.DefaultHttp2Connection.notifyClosed(DefaultHttp2Connection.java:357)
                class: io.netty.handler.codec.http2.Http2Connection$Listener
                count: 6228227
        io.netty.channel.AbstractChannelHandlerContext.invokeRead(AbstractChannelHandlerContext.java:839)
                class: io.netty.channel.ChannelOutboundHandler
                count: 5488418
        io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
                class: io.netty.channel.ChannelInboundHandler
                count: 5343287
        io.netty.channel.AbstractChannelHandlerContext.invokeChannelReadComplete(AbstractChannelHandlerContext.java:486)
                class: io.netty.channel.ChannelInboundHandler
                count: 5172992
        io.netty.handler.codec.http2.DefaultHttp2Connection.removeStream(DefaultHttp2Connection.java:317)
                class: io.netty.handler.codec.http2.Http2Connection$Listener
                count: 1525703
        io.netty.handler.codec.http2.DefaultHttp2Connection$ActiveStreams.addToActiveStreams(DefaultHttp2Connection.java:995)
                class: io.netty.handler.codec.http2.Http2Connection$Listener
                count: 1145767
        io.netty.channel.AbstractChannelHandlerContext.invokeChannelInactive(AbstractChannelHandlerContext.java:305)
                class: io.netty.channel.ChannelInboundHandler
                count: 1
        io.netty.channel.ChannelHandlerMask.mask0(ChannelHandlerMask.java:123)
                class: io.netty.channel.ChannelOutboundHandler
                count: 1
        io.netty.handler.codec.http2.DefaultHttp2Connection.goAwayReceived(DefaultHttp2Connection.java:237)
                class: io.netty.handler.codec.http2.Http2Connection$Listener
                count: 1
        io.netty.handler.codec.http2.DefaultHttp2ConnectionEncoder.lifecycleManager(DefaultHttp2ConnectionEncoder.java:62)
                class: io.netty.handler.codec.http2.Http2LifecycleManager
                count: 1
        io.netty.channel.ChannelHandlerMask.mask0(ChannelHandlerMask.java:94)
                class: io.netty.channel.ChannelInboundHandler
                count: 1
--------------------------```

The first one in the chart (not reported) is a false positive (meaning that we have a single concrete class in all the relevant code paths making the issue to disappear, optimized away by the JIT): this is is why I'm reporting the affected classes starting from 2.

This case instead is real, awaiting to be triggered by "normal" and possible runtime condititions

eg
If the user/vertx setup a pipeline which contains 1 more io.netty.channel.ChannelInboundHandler-only or io.netty.channel.ChannelOutboundHandler-only types, which will hit:

        io.netty.channel.AbstractChannelHandlerContext.invokeRead(AbstractChannelHandlerContext.java:839)
                class: io.netty.channel.ChannelOutboundHandler
                count: 5488418
        io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
                class: io.netty.channel.ChannelInboundHandler
                count: 5343287
        io.netty.channel.AbstractChannelHandlerContext.invokeChannelReadComplete(AbstractChannelHandlerContext.java:486)
                class: io.netty.channel.ChannelInboundHandler
                count: 5172992

other than just io.vertx.core.http.impl.VertxHttp2ConnectionHandler'; this will make the type checks there for this class to happen for real, causing the klass's secondary_super_cache to ping pong among these types, and hitting the issue at netty/netty#12806 (comment)

In short, the problem arise because io.vertx.core.http.impl.VertxHttp2ConnectionHandler is extending Http2ConnectionHandler's which is transitively implementing both inbound/outbound handlers.

image

As usual; this seems a kind of fix which should happen within Netty, but I'm reporting here because is directly affecting us and Netty's Http2ChannelDuplexHandler TBH, being a duplex handler, doesn't seem affect (last famous words...).

Additionally, we do have other interfaces which the IntelliJ diagram seems to have forgotten (it's a bug???) ie Http2FrameListener and Http2Connection.Listener , but only the latter appear in the agent's report.

It doesn't seem harmful because the only implementor of Http2Connection$Listener seems VertxHttp2ConnectionHandler apart from an implementor within Netty which we don't use (because we don't use Http2ChannelDuplexHandler).

@franz1981
Copy link
Contributor Author

I've opened netty/netty#13741 to mitigate this, but will likely solve just the ChannelinputHandler part, while the io.netty.handler.codec.http2.Http2Connection$Listener, one, not yet, although this last one could be just a false-positive

@franz1981
Copy link
Contributor Author

Thanks to netty/netty#13741 in, I can just send a pr to fix the listener case at #5039 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant