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

Reduce chances of type check scalability issues (Fixes #5039) #5047

Merged
merged 1 commit into from
Dec 21, 2023

Conversation

franz1981
Copy link
Contributor

@franz1981 franz1981 commented Dec 21, 2023

This is performing 2 changes:

      io.netty.handler.codec.http2.DefaultHttp2Connection$DefaultEndpoint.addStream(DefaultHttp2Connection.java:805)
                class: io.netty.handler.codec.http2.Http2Connection$Listener
                count: 7912545

matches with https://github.com/netty/netty/blob/013c38298f6d28ec14543d02ede9bb6a6472ca3f/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java#L805
which listeners collections contains:

0 = {WeightedFairQueueByteDistributor$1@5819} 
1 = {DefaultHttp2RemoteFlowController$1@5855} 
2 = {DefaultHttp2LocalFlowController$1@5856} 
3 = {VertxHttp2ConnectionHandler@5851} 

meaning that the checkcast hidden in the ArrayList::get(int) (because of its generic type), will observe 4 different concrete types, causing the checkcast against to likely happen, although the JDK 22 JIT optimization mentioned at netty/netty#13745 (comment)

  • Improve the fast-path check vs ByteBuf while reading the data passed by the pipeline, as reported by
--------------------------
9:      io.netty.buffer.PooledUnsafeDirectByteBuf
Count:  5541640
Types:
        io.netty.handler.codec.http2.Http2StreamFrame
Traces:
        io.vertx.core.http.impl.VertxHttp2ConnectionHandler.channelRead(VertxHttp2ConnectionHandler.java:405)
                class: io.netty.handler.codec.http2.Http2StreamFrame
                count: 5541640
--------------------------

The first is effective since netty/netty#13741 has already been merged and it removes the ChannelInboundHandler case in the report.

With this PR there should be a single type check at

        io.netty.channel.AbstractChannelHandlerContext.invokeRead(AbstractChannelHandlerContext.java:839)
                class: io.netty.channel.ChannelOutboundHandler
                count: 5488418

which won't flip-flop its type anymore, leveraging the existing secondary_super_cache value in VertxHttp2ConnectionHandler's Klass.

@vietj vietj merged commit e66fe44 into eclipse-vertx:4.x Dec 21, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants