Cache lookups that yield a null HeaderDelegate

[geoand]

This is meant to address part of quarkusio/quarkus#4345

[geoand]

Oh darn... something happened with the line endings...

[geoand]

Line endings fixed

[asoldano]

@geoand thanks for the PR. This is the type of things that I like (performance optimizations ;-) ), but which also need some confirmation numbers in a benchmark. Do you have a plan for running comparisons? Otherwise I'll see if we can adapt one of the internal bench tests we have (based on WildFly)

[geoand]

@asoldano I did run some benchmarks with this in fact.

It's basically a Quarkus application (see this PR) that is load tested with wrk.

Before the patch I (well actually @Sanne was the first to report it) was seeing (from async-profiler's alloc type profiling) allocations like the following:

--- 2205236646496 bytes (2.20%), 72 samples
  [ 0] java.lang.Class[]
  [ 1] java.lang.Class.getInterfaces
  [ 2] org.jboss.resteasy.core.providerfactory.Utils.createHeaderDelegateFromInterfaces
  [ 3] org.jboss.resteasy.core.providerfactory.Utils.createHeaderDelegate
  [ 4] org.jboss.resteasy.core.providerfactory.ResteasyProviderFactoryImpl.createHeaderDelegate
  [ 5] org.jboss.resteasy.core.providerfactory.ResteasyProviderFactoryImpl.getHeaderDelegate
  [ 6] io.quarkus.resteasy.runtime.standalone.VertxHttpResponse.transformHeaders
  [ 7] io.quarkus.resteasy.runtime.standalyone.VertxHttpResponse.writeBlocking
  [ 8] io.quarkus.resteasy.runtime.standalone.VertxOutputStream.close
  [ 9] org.jboss.resteasy.util.CommitHeaderOutputStream.close
  [10] io.quarkus.resteasy.runtime.standalone.VertxHttpResponse.finish
  [11] io.quarkus.resteasy.runtime.standalone.VertxRequestHandler.dispatch
  [12] io.quarkus.resteasy.runtime.standalone.VertxRequestHandler.dispatchRequestContext
  [13] io.quarkus.resteasy.runtime.standalone.VertxRequestHandler.lambda$handle$0
  [14] io.quarkus.resteasy.runtime.standalone.VertxRequestHandler$$Lambda$267.629030423.handle
  [15] io.vertx.core.impl.ContextImpl.lambda$executeBlocking$2
  [16] io.vertx.core.impl.ContextImpl$$Lambda$271.2098082615.run
  [17] java.util.concurrent.ThreadPoolExecutor.runWorker
  [18] java.util.concurrent.ThreadPoolExecutor$Worker.run
  [19] io.netty.util.concurrent.FastThreadLocalRunnable.run
  [20] java.lang.Thread.run

I should note that this was the top allocation contributor in that benchmark.

After the patch was applied allocation of java.lang.Class[] were no longer coming from this method (and had dropped to 0.28% in total).

Running the same type scenario using JFR I was seeing java.lang.Class[] allocations drop to about a third in amount of memory used.

Also in my tests requests/sec didn't really change with this patch.

[geoand]

@asoldano do you need any more benchmarking data for this?

[Sanne]

@asoldano the performance improvements of this are substantial - but as usual they will depend on circumstances. With the benchmark I'm working on, the dominating cost is allocations rate - and this addressed it directly so I can see a substantial improvement.

Yet of course if you have a different benchmark which is not dominated by this cost, I guess you'll see only negligible / zero improvements.

+1 to accept it please :)

One thing I'm not sure of, as I don't know the lifecycle of this component, is if it's safe to store Class instances in that set - horrible memories of classloader leaks - but I guess it is as you already have another map doing the same, which makes me thing that either you have an existing leak, or the scope of this component is bound to a single deployment.

[asoldano]

Merged, thanks @geoand and @Sanne

[geoand]

Thanks for reviewing @asoldano!