RFR - JDK-8218650: LineNumberTable records for method invocations with arguments

[Eddie Aftandilian]

Sorry for the delay, I got pulled off onto other tasks for a while.

Vicente said:
> I'm not opposed to adding more lines to the LNT if that's what you are
proposing, but I wonder about the benefit. I mean how bad it is to miss for
one, two lines vs having bigger class files. Actually, this is an
interesting experiment @Eddie could you please run an experiment to see how
bigger would the class files for the whole JDK be with your patch? I'm also
interested in redundant lines, it could be that this code adds redundant
lines to the LNT and we should avoid that

I ran the experiment on the class file sizes for the JDK:

Without patch:
eaftan at eaftan:~/openjdk-source/jdk/build/linux-x86_64-server-release/jdk$
find -name *.class -type f | xargs du -cb
2929167 total

With patch:
eaftan at eaftan:~/openjdk-source/jdk/build/linux-x86_64-server-release/jdk$
find -name *.class -type f | xargs du -cb
2929631 total

Difference: +0.0158407%

This seems suspiciously small, but I confirmed by inspection that the javac
patch had the expected effect.  For example, when looking at the LNT for
com.sun.crypto.provider.AESCipher#getEngineKeySize, in the patched version
I see a new LNT entry `line 510: 32`, where the relevant source lines are

```
509             throw new InvalidKeyException("Invalid AES key length: " +
510                                           encoded.length + " bytes");
```

So it seems the impact on class file size is minimal.

Re: redundant lines in the LNT -- my (poor) understanding of the javac code
that emits LNT entries is that it tries to prevent this.  Any suggestions
on how to determine whether there are redundant LNT entries being emitted?

-Eddie

On Thu, Feb 21, 2019 at 3:37 PM Maurizio Cimadamore <
maurizio.cimadamore at oracle.com> wrote:

>
> On 21/02/2019 19:27, Liam Miller-Cushon wrote:
>
> For what it's worth I filed JDK-8218650 after a real debugging session
> where I spent a while trying to understand an 'impossible' NPE before
> realizing it was actually happening on the next line. So anecdotally at
> leave one moderately savvy consumer of debug info would have benefited from
> the extra LNT entries :)
>
> The approach in JDK-8218628 sounds very promising, though. That
> information would have been sufficient in the example that motivated
> JDK-8218650.
>
> > e.g. access to fields in different classes with same name :-)
>
> Won't that be an issue even if javac starts emitting LNT entries for field
> accesses *and* the NPE messages get more detailed?
>
> i.e. you could also have `b.b.b` and even if we know the line and that `b`
> was involved it's ambiguous which dereference failed. (If the NPE message
> included a qualified name of `b` that would be sufficient except for some
> really pathological cases.)
>
> Yes - in that case not much can be done - unless we add column info too :-)
>
> Another option for improving the message for `b.b.b` would be to include a
> column number, which was discussed in JDK-8020204.
>
> Ah :-)
>
> Maurizio
>
>
> On Thu, Feb 21, 2019 at 10:59 AM Maurizio Cimadamore <
> maurizio.cimadamore at oracle.com> wrote:
>
>> Yeah - I was afraid that will be the case.
>>
>> And I'm wondering what happens in cases where you have stuff like
>>
>> b.
>>    b.
>>       b.
>>
>> e.g. access to fields in different classes with same name :-)
>>
>> Of course a corner case, but...
>>
>> Maurizio
>>
>>
>> On 21/02/2019 17:38, Vicente Romero wrote:
>> > but it still refers to line 13, so it doesn't seem like it will
>> > produce an exact reference to the actual line where the NPE happened
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20190308/56e5e4ea/attachment-0001.html>