RFR(S) 8218751 Do not store original classfiles inside the CDS archive

[Ioi Lam]

Hi Jiangli,

The main reason for doing this is to reduce the size of the CDS file. 
For example, when archiving Eclipse IDE with JDK-8215311 (Dynamic Class 
Metadata Archive), the CDS archive is reduced from 100MB to about 67MB.

We have not heard any requirement for high performance with 
CDS+ClassFileLoadHook. It doesn't seem right for everyone to take a 50% 
file size penalty for an optimization that no one has asked for.

Here are some perf numbers (running "java -version" with an agent that 
installs a CFLH that doesn't nothing). Yes, it's somewhat slower as 
expected, but it doesn't seem to be catastrophic. Using CDS is 
nevertheless much faster than without CDS anyway.

After No CFLH         0.0476 seconds time elapsed  ( +- 0.20% )
After with CFLH       0.0513 seconds time elapsed  ( +- 0.18% )  7.773% 
slower

Before no CFLH        0.0472 seconds time elapsed  ( +- 0.21% )
Before with CFLH      0.0492 seconds time elapsed  ( +- 0.18% )  4.237% 
slower

CDS Off with CFLH     0.0869 seconds time elapsed  ( +- 0.18% )
CDS Off without CFLH  0.0852 seconds time elapsed  ( +- 0.14% )  1.995% 
slower

I am not sure if there's a performance critical case for CFLH. I suppose 
if performance is critical you would rewrite the classes statically and 
rebuild your app, instead of patching its bytecodes at runtime. However, 
if there were indeed a performance critical case, I think it's better to 
change JVMTI to allow a 2-level filtering for CFLH:

I believe the overwhelming use case, where performance is critical, the 
CFLH will just patch a small number of class files. I can't fathom any 
use case where someone wants to patch EVERY loaded class.

The current CFLH passes both the name of the class, as well as the 
classfile data, in a single call. This forces CDS to decode the 
classfile data for every hook call. However, in most cases, the CFLH 
will just examine the class name, and do nothing unless the name matches 
a certain pattern, so we end up wasting the decoding effort.

My suggested improvement is to add a new filtering call in JVMTI that 
passes only the name. If the CFLH wants to patch the class, it will then 
request the classfile data, at which point CDS will decode it from the 
modules file or JAR file.

Thanks

- Ioi


On 2/12/19 5:18 PM, Jiangli Zhou wrote:
> Hi Ioi,
>
> I'd like to understand the performance impact with this change. Do you have
> any performance numbers  when JvmtiExport::should_post_class_file_load_hook()
> is required? This is a performance vs footprint trade-off. For some users,
> performance is more important than static footprint.
>
> Could you also please provide some background/motivation for this change?
>
> Thanks,
> Jiangli
>
> On Mon, Feb 11, 2019 at 9:24 AM Ioi Lam <ioi.lam at oracle.com> wrote:
>
>> http://cr.openjdk.java.net/~iklam/jdk13/8218751-dont-store-classfiles-in-cds.v01/
>> https://bugs.openjdk.java.net/browse/JDK-8218751
>>
>> For JVMTI ClassFileLoadHook support, the CDS archive currently stores
>> the original classfile data of all archived classes.
>>
>> However, this consists of over 30% of the archive size. Because all
>> original classfile data are already available in other files (such as the
>> JDK lib/modules file, or JAR files in the classpath), we can simply read
>> from these locations when needed by JVMTI.
>>
>> For the default CDS archive (included as part of the JDK distribution),
>> the size is reduced from about 18.5MB to 12.1MB on Linux/x64.
>>
>> Thanks
>>
>> - Ioi
>>
>>