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

[Ioi Lam]

Hi Calvin

Thanks for the review.

On 2/12/19 11:29 AM, Calvin Cheung wrote:
> Hi Ioi,
>
> The following call is probably slower than before because it involves 
> reading from a jar file.
> cfs = FileMapInfo::open_stream_for_jvmti(ik, CHECK_NULL);
> It is probably ok since the above is called under the following 
> condition:
> if (JvmtiExport::should_post_class_file_load_hook())
>
I think the slow down is probably going to be OK. If not, some users 
will tell us :-) I remember the old days when CDS will be completely 
disabled when CFLH is used, but no one was complaining at that time.


> Minor comments in the following 2 files:
>
> filemap.cpp
> 1494   assert(path_index >= 0, "should be called for shared built-in 
> classes only");
>
> Should the above be also checking (path_index < 
> ClassLoaderExt::app_class_paths_start_index())?
>
I already have this check

   assert(path_index < (int)_shared_path_table_size, "sanity");

It's possible for path_index to be >= 
ClassLoaderExt::app_class_paths_start_index(). For example, if the class 
is being loaded from the app classpath (e.g., hello.jar when loading an 
archived HelloWorld class).


> SpaceUtilizationCheck.java
> 106           throw new RuntimeException("Must have 5 consecutive, 
> fully utilized regions");
>
> 5 should be 4
>
Fixed.

Thanks

- Ioi


> thanks,
> Calvin
>
> On 2/11/19, 9:23 AM, Ioi Lam 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
>>