Questions about using `assert` in Java

[John Rose]

On 17 Jul 2023, at 2:08, Alan Bateman wrote:

> On 15/07/2023 17:53, Daohan Qu wrote:
>
> You will find places in the JDK code, esp. in performance critical 
> code, where assertions are commented out. The reason is that asserts, 
> even if disabled, increase the method size and can impact inlining by 
> the compiler at run-time.  So while useful when debugging some issue 
> in such code, they are commended out to avoid increasing the method 
> size.

In JDK code that must be hand-tuned for performance, small methods are 
sometimes broken into 2-3 even smaller methods.  This is rarely done, 
because it is a compromise against maintainability.  It can provide a 
way to introduce asserts even into performance sensitive methods, if 
there is some particular need.

The basic idea is to have the assert expression be a call to a private 
static helper method, so its code size is minimal.  Then, if necessary, 
have the actual computation (before or after the assert) moved into a 
second helper method.

Here’s an example.  (It doesn’t do anything interesting, but it has 
a few mildly complicated expressions.)

```
class AssertExample {
     public static double circleQueryFast(double x, double y) {
         assert circleQueryChecks(x, y);
         return circleQueryCompute(x, y);
     }
     private static boolean circleQueryChecks(double x, double y) {
         return x*x + y*y < 1.0;
     }
     private static double circleQueryCompute(double x, double y) {
         double x2 = x*x, y2 = y*y;
         return x2 < y2 ? y - x : x - y;
     }
     public static double circleQuerySlow(double x, double y) {
         double x2 = x*x, y2 = y*y;
         assert x2 + y2 < 1.0;
         return x2 < y2 ? y - x : x - y;
     }
}
```

In this example, the “fast” public method will often inline and 
optimize better than the “slow” public method.  This is because the 
all-in-one “slow” method is rather large, and the other three, 
although they collectively are even larger, are each simple enough to 
get a very favorable rating from the inlining policy.

Specifically, if a method has 35 bytecodes or less it gets favorable 
inlining decision.  (That is, `MaxInlineSize=35`.)  All of the above 
methods except the “slow” one are within this limit.  (Try `javac 
AssertExample.java  && javap -c -p AssertExample.class`.)

That might seem an outrageous oversimplification, but if you think about 
it, the inlining policy of an online JIT needs to be relatively simple, 
so it can make its decisions very quickly.  Getting a more balanced 
decision here would require something like a backtracking inlining 
policy, or a search over potential call graphs.  That is certainly 
possible, but it was not the practical decision 25 years ago when this 
inlining policy was designed.

Should we fix it?  Well, yes, and there are RFEs to work on for this:

https://bugs.openjdk.org/browse/JDK-6445664

But it’s not just a matter of filing RFEs or willpower or resources; 
there are risks due to the scale of Java code already deployed on 
HotSpot.  Anything we do now, after years of people tuning their Java 
code against this (implicit) contract, had better not make the existing 
code run much slower.  Or maybe there could be some user-friendly path 
to detection and remediation, perhaps using diagnostic flags.

For asserts, I think it would be relatively safe and efficient to remove 
the effects of untaken blocks (including assert logic) from the metric 
that MaxInlineSize is compared against.

That is, since an assert compiles to an if/then construct, where the 
if-condition is a constant which is usually false (only true under `java 
-ea`), it would be reasonable to note somehow (at inline policy time) 
that the relevant if/then-block will never be taken.  This analysis 
could be gated by profile information, at least in part.  The JIT does 
this sort of thing anyway, but it is not connected fully to the inlining 
policy, at present.

But this will require some research.  It could perturb even existing 
programs, with a certain number of them having a slight loss of 
performance.  The C2 inlining policy has become almost fossilized by its 
own success.  It’s easy to show cases where it gives the wrong answer, 
but hard to show an alternative that would do no harm to the millions of 
workloads out there.

— John
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/core-libs-dev/attachments/20230717/e14317d5/attachment.htm>