RFR: 8273563: Improve performance of implicit exceptions with -XX:-OmitStackTraceInFastThrow [v2]

[David Holmes]

Please ignore - I deleted this comment as I realized what I was missing 
(the '-' sign) as soon as I posted it. :(

David

On 21/09/2021 3:12 pm, David Holmes wrote:
> On Thu, 16 Sep 2021 17:00:20 GMT, Volker Simonis <simonis at openjdk.org> wrote:
> 
>>> Currently, if running with `-XX:-OmitStackTraceInFastThrow`, C2 has no possibility to create implicit exceptions like AIOOBE, NullPointerExceptions, etc. in compiled code. This means that such methods will always be deoptimized and re-executed in the interpreter if such exceptions are happening.
>>>
>>> If implicit exceptions are used for normal control flow, that can have a dramatic impact on performance. A prominent example for such code is [Tomcat's `HttpParser::isAlpha()` method](https://github.com/apache/tomcat/blob/26ba86cdbd40ca718e43b82e62b3eb49d004c3d6/java/org/apache/tomcat/util/http/parser/HttpParser.java#L266-L274):
>>>
>>>      public static boolean isAlpha(int c) {
>>>          try {
>>>              return IS_ALPHA[c];
>>>          } catch (ArrayIndexOutOfBoundsException ex) {
>>>              return false;
>>>          }
>>>      }
>>>
>>>
>>> ### Solution
>>>
>>> Instead of deoptimizing and resorting to the interpreter, we can generate code which allocates and initializes the corresponding exceptions right in compiled code. This results in a ten-times performance improvement for the above code:
>>>
>>> -XX:-OmitStackTraceInFastThrow -XX:-OptimizeImplicitExceptions
>>> Benchmark                 (exceptionProbability)  Mode  Cnt      Score      Error  Units
>>> ImplicitExceptions.bench                     0.0  avgt    5      1.430 ±    0.353  ns/op
>>> ImplicitExceptions.bench                    0.33  avgt    5   3563.038 ±   77.358  ns/op
>>> ImplicitExceptions.bench                    0.66  avgt    5   8609.693 ± 1205.104  ns/op
>>> ImplicitExceptions.bench                    1.00  avgt    5  12842.401 ± 1022.728  ns/op
>>>
>>> -XX:-OmitStackTraceInFastThrow -XX:+OptimizeImplicitExceptions
>>> Benchmark                 (exceptionProbability)  Mode  Cnt      Score      Error  Units
>>> ImplicitExceptions.bench                     0.0  avgt    5     1.432  ±    0.352  ns/op
>>> ImplicitExceptions.bench                    0.33  avgt    5   355.723  ±   16.641  ns/op
>>> ImplicitExceptions.bench                    0.66  avgt    5   887.068  ±  166.728  ns/op
>>> ImplicitExceptions.bench                    1.00  avgt    5  1274.418  ±   88.235  ns/op
>>>
>>>
>>> ### Implementation details
>>>
>>> - The new optimization is guarded by the option `OptimizeImplicitExceptions` which is on by default.
>>> - In `GraphKit::builtin_throw()` we can't simply use `CallGenerator::for_direct_call()` to create a `DirectCallGenerator` for the call to the exception's `<init>` function because `DirectCallGenerator` assumes in various places that calls are only issued at `invoke*` bytecodes. This is is not true in genral for bytecode which can cause an implicit exception.
>>> - Instead, we manually wire up the call based on the code in `DirectCallGenerator::generate()`.
>>> - We use a similar trick like for method handle intrinsics where the callee from the bytecode is replaced by a direct call and this fact is recorded in the call's `_override_symbolic_info` field. For calling constructors of implicit exceptions I've introduced the new field `_implicit_exception_init`. This field is also used in various assertions to prevent queries for the bytecode's symbolic method information which doesn't exist because we're not at an `invoke*` bytecode at the place where we generate the call.
>>> - The PR contains a micro-benchmark which compares the old and the new implementation for [Tomcat's `HttpParser::isAlpha()` method](https://github.com/apache/tomcat/blob/26ba86cdbd40ca718e43b82e62b3eb49d004c3d6/java/org/apache/tomcat/util/http/parser/HttpParser.java#L266-L274). Except for the trivial case where the exception probability is 0 (i.e. no exceptions are happening at all) the new implementation is about 10 times faster.
>>
>> Volker Simonis has updated the pull request incrementally with one additional commit since the last revision:
>>
>>    Minor updates as requested by @TheRealMDoerr
> 
> I think I must have a basic misunderstanding of the problem here, as the described problem seems to be the opposite of what the intent of OmitStackTraceInFastThrow actually is. From the comment in graphKit:
> 
> // If this throw happens frequently, an uncommon trap might cause
> // a performance pothole.  If there is a local exception handler,
> // and if this particular bytecode appears to be deoptimizing often,
> // let us handle the throw inline, with a preconstructed instance.
> 
> so OmitStackTraceInFastThrow actually allows us to use an optimized fastpath because we can replace a heavyweight stack-full exception with a preallocated stackless one and so avoid the uncommon trap to create the exception.
> 
> What am I missing?
> 
> Thanks,
> David
> 
> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/5488
>