smallrye / smallrye-common

Common utilities for SmallRye
Apache License 2.0
21 stars 24 forks source link

Avoid triggering type cache pollution on VertxContext.isDuplicatedContext #190

Closed Sanne closed 2 years ago

Sanne commented 2 years ago

This is a strong performance enhancement. Using @franz1981's new agent we can now identify suboptimal code in our libraries when doing instanceof (or comparable, such as casts) operations which trigger JDK-8180450.

It would seem we have one of such cases in method VertxContext.isDuplicatedContext , the related portion of the report being:

--------------------------
2:  io.vertx.core.impl.DuplicatedContext
Count:  24323910
Types:
    io.vertx.core.impl.ContextInternal
    io.vertx.core.Context
Traces:
    io.smallrye.common.vertx.VertxContext.isDuplicatedContext(VertxContext.java:114)
        class: io.vertx.core.Context
        count: 11809285
    io.smallrye.common.vertx.VertxContext.isDuplicatedContext(VertxContext.java:115)
        class: io.vertx.core.impl.ContextInternal
        count: 10516203
    io.vertx.core.http.impl.Http1xServerConnection.handleMessage(Http1xServerConnection.java:135)
        class: io.vertx.core.impl.ContextInternal
        count: 509081
    io.quarkus.vertx.core.runtime.VertxCoreRecorder$14.runWith(VertxCoreRecorder.java:561)
        class: io.vertx.core.impl.ContextInternal
        count: 505264
    io.vertx.ext.web.impl.RoutingContextImpl.getEndHandlers(RoutingContextImpl.java:508)
        class: io.vertx.core.impl.ContextInternal
        count: 494211
    io.vertx.core.streams.impl.InboundBuffer.<init>(InboundBuffer.java:95)
        class: io.vertx.core.impl.ContextInternal
        count: 489866
--------------------------

As one can see, the common pattern is to cast an instance of io.vertx.core.impl.ContextInternal to io.vertx.core.impl.ContextInternal; there's only one exception here perturbing the cache: this method is implicitly casting to its obvious interface io.vertx.core.Context.

The fix is simple: consistently cast to io.vertx.core.impl.ContextInternal exclusively.

radcortez commented 2 years ago

@Sanne, you should be signing your commits :)