Refactor getMethodIfAvailable to not cause NoSuchMethodExceptions

[philwebb]

Refactor ClassUtils.getMethodIfAvailable(...) to loop over methods
rather than cause a NoSuchMethodException. Since this method is called
often, this change can help improve application startup times.

Issue:

[dreis2211]

May I suggest a pre-check for getParameterCount() == paramTypes.length in order to avoid possible allocations coming from getParameterTypes()?

[philwebb]

Good call. Will rework the PR. Thanks!

[dreis2211]

Hi @philwebb,

I took the freedom to do some benchmarking with roughly the following benchmark:

    @Benchmark
    public Method testNew(TestState testState) {
        return ClassUtilsNew.getMethodIfAvailable(Collection.class, "remove", testState.params);
    }

    @Benchmark
    public Method testOld(TestState testState) {
        return ClassUtils.getMethodIfAvailable(Collection.class, "remove", testState.params);
    }

The results below show that the new approach is considerably slower than sticking with the exception (though I wish one could get rid of this overhead):

Benchmark                                                             Mode  Cnt        Score         Error   Units
MyBenchmark.testNew                                                  thrpt   10  1205631,942 ±   19528,209   ops/s
MyBenchmark.testNew:·gc.alloc.rate                                   thrpt   10     2160,500 ±      34,985  MB/sec
MyBenchmark.testNew:·gc.alloc.rate.norm                              thrpt   10     1880,001 ±       0,001    B/op
MyBenchmark.testNew:·gc.churn.CodeHeap_'non-profiled_nmethods'       thrpt   10        0,004 ±       0,004  MB/sec
MyBenchmark.testNew:·gc.churn.CodeHeap_'non-profiled_nmethods'.norm  thrpt   10        0,004 ±       0,003    B/op
MyBenchmark.testNew:·gc.churn.G1_Old_Gen                             thrpt   10     2155,113 ±     106,008  MB/sec
MyBenchmark.testNew:·gc.churn.G1_Old_Gen.norm                        thrpt   10     1875,308 ±      86,973    B/op
MyBenchmark.testNew:·gc.count                                        thrpt   10       91,000                counts
MyBenchmark.testNew:·gc.time                                         thrpt   10       74,000                    ms
MyBenchmark.testOld                                                  thrpt   10  8600278,472 ± 4189300,388   ops/s
MyBenchmark.testOld:·gc.alloc.rate                                   thrpt   10      918,050 ±     447,475  MB/sec
MyBenchmark.testOld:·gc.alloc.rate.norm                              thrpt   10      112,000 ±       0,001    B/op
MyBenchmark.testOld:·gc.churn.CodeHeap_'non-profiled_nmethods'       thrpt   10        0,003 ±       0,003  MB/sec
MyBenchmark.testOld:·gc.churn.CodeHeap_'non-profiled_nmethods'.norm  thrpt   10       ? 10?³                  B/op
MyBenchmark.testOld:·gc.churn.G1_Old_Gen                             thrpt   10      926,888 ±     446,573  MB/sec
MyBenchmark.testOld:·gc.churn.G1_Old_Gen.norm                        thrpt   10      113,956 ±      13,283    B/op
MyBenchmark.testOld:·gc.count                                        thrpt   10       66,000                counts
MyBenchmark.testOld:·gc.time                                         thrpt   10       49,000                    ms

This mainly comes from the additional heap-pressure as the JDK is copying the method array etc.

The results above make me wonder if this will actually improve the performance.

Cheers,
Christoph

[philwebb]

@dreis2211 Thanks for writing those benchmarks. I think you're right, running my samples again with the changes reverted showed no difference in startup time. I was convinced that it had made a difference but I guess not.

I wish there was a quick way to tell if a method existed without the exception, but I guess it's not to be.