perf: probing access without exception, cache MethodHandles

[jbee]

Background

I ran generating OpenAPI HTML with profiling on. Interestingly the most expensive methods had to do with method handle resolve and filling in stack trace information of thrown exceptions. It was clear that something that happens very often must throw exceptions and that the method handle resolving was either slow itself or also did throw exceptions.

Reasoning

I knew that the JsonValue API does some of its evaluations by trying to access a node and then catch the potential exception in case the node did not exist. This was an obvious candidate for being responsible for countless exceptions. So I created alternative methods that would not throw an exception but return null instead.

Secondly I added caching to the method handle lookup. I also found when trying my improvements in the profiler that the method handle resolution itself was one source of throwing and then catching an exception. So caching that avoided doing throw + catch a lot.

This image shows that vast amount of time was spend in fillInStackTrace and MethodHandleNatives.resolve and MethodHandleNatives.init. After these changes those rank far lower with times in the hundredths of ms not thousands (of the total time it took to generate the OpenAPI HTML)

Based on the methods highlighted by the profiler I also had a look at the skip-methods and simplified them a bit to allow better in-lining in case that was a reason for them ranking high and if looking at the numbers it did indeed improve noticeably.

Summary

• feat: adds getOrNull, elementOrNull and memberOrNull methods to JsonNode to avoid exceptions when checking
• perf: remember size of objects and arrays at the end of iterating their children
• perf: adds early exit path for child lookup - if a child is not in the cache but size is known we can infer that all children are cached so the child we are looking for does not exist
• perf: size() first uses size field if availanle
• perf: MethodHandles are now cached (avoid internal exceptions)
• perf: handling of paths as String which checks for a handful of special cases now compares a char instead of a String
• perf: when invoking a method handle with no arguments invoke is used instead of invokeWithArguments (which does much more adjustment work that is not needed without arguments)
• perf: avoid lambdas in skipWhitespace as it is used a lot

Automatic Testing

I added some basic test for getOrNull. This internally uses elementOrNull and memberOrNull. The new test scenarios only test the "not existing" case since the positive case is tested plenty indirectly by lots of other tests as the JsonValue implementation now mostly uses getOrNull. But some methods sill use get as they want exceptions so both essential methods are still well tested.

JDK

As far as I can tell the exception thrown as part of resolving a MethodHandle is in MemberName#resolve https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/invoke/MemberName.java#L957 which is caught https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/lang/invoke/MemberName.java#L963 The exact exception thrown is a IncompatibleClassChangeError with the message "Found class java.lang.Object, but interface was expected". But when putting a breakpoint at the error constructor multiple other cases for invoke static method handles also occur which never escape the method handle lookup itself (code has the intended result). I would assume that all these cases not necessarily have to cause throw + catch and could equally be tested for using a if with the equivalent condition somewhere on the outside.

[jbee]

The getOrNull methods that only exist to throw exceptions (never returning their return type) don't make sense to someone who doesn't know anything about this library :)

Oh it does return null where get would throw the JsonPathException. Or maybe I misunderstand what you mean.

[david-mackessy]

The getOrNull methods that only exist to throw exceptions (never returning their return type) don't make sense to someone who doesn't know anything about this library :)

Oh it does return null where get would throw the JsonPathException. Or maybe I misunderstand what you mean.

Like this method:

@Maybe
default JsonNode getOrNull(@Surly JsonPath path) throws JsonPathException {
    throw new JsonPathException( path,
        format( "This is a leaf node of type %s that does not have any children at path: %s", getType(), path ) );
}

It looks like it will always throw an exception and never return a JsonNode. Does this result in null being returned somehow?

[jbee]

Ah! yes. It might still be uncommon to use default methods this much but this is the same old pattern that people have used with abstract classes where those subtypes that implement a method do override it. I am sure this will become more and more common practice once more projects discover how neat defaults methods are :D

[david-mackessy]

Ah! yes. It might still be uncommon to use default methods this much but this is the same old pattern that in people have used with abstract classes where those subtypes that implement a method do override it. I am sure this will become more and more common practice once more projects discover how neat defaults methods are :D

Ah, ok. Looking at the code in a PR (not in IDE) makes it easy to miss things like this :)