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

[Vicente Romero]

Hi Eddie,

Thanks for the additional experiments, it is good to know that there is 
not too much increase in the class file size after the patch

On 3/8/19 8:41 PM, Eddie Aftandilian wrote:
> 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
> 2929167total
>
> With patch:
> eaftan at eaftan:~/openjdk-source/jdk/build/linux-x86_64-server-release/jdk$ 
> find -name *.class -type f | xargs du -cb
> 2929631total
>
> 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?

regarding this I think that the easiest way could be to determine what 
class files have different size, and get a set of them, produce the 
javap output and compare them. You should see the changes in the LNT and 
if there are redundant entries.

>
> -Eddie

Vicente

>
> On Thu, Feb 21, 2019 at 3:37 PM Maurizio Cimadamore 
> <maurizio.cimadamore at oracle.com 
> <mailto: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
>>     <mailto: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/20190311/351be979/attachment.html>