[jdk11u-dev] RFR: 8261090: Store old classfiles in static CDS archive [v2]

[Thomas Stuefe]

On Fri, 11 Mar 2022 03:24:54 GMT, wxiang <duke at openjdk.java.net> wrote:

>> wxiang has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   revert the changes with 8266822 and 8267431
>
> Thanks a lot for the suggestion.
> As described in JDK-8230413, there are still large number of existing classes with older class version (< 50) in real world applications. Those classes are missing the benefit of CDS. For better use of AppCDS, we requested the backport.
> We have run all the test cases under hotspot/jtreg/runtime/appcds includes these new cases in this patch, 
> and run some demos which has old classifies, such as spring-petclinic.
> We have submitted a new patch to revert the change for for JDK-8266822 and JDK-8267431

Hi @shiyuexw,

Decision is not up to me, so just my personal opinion.  But I have strong reservations about downporting this patch:

- it is large and complex. It's a lot of Reviewer work taken away from other issues.
- its effects are difficult to estimate. Even if this patch was working flawlessly, we'd now run with a much larger number of old classes getting stored in application archives, which means CDS and the VM runtime exercised in a lot new ways. This may destabilize 11u by shaking bugs lose we have not encountered yet or may have not been down ported yet. 
- It's also difficult to test. Because the real behavior difference will show up only in applications using large amounts of very old class files <JDK6. To get good test coverage we'd need some large scale tests for scenarios like that.
- Since Oracle decided to not downport it in its closed 11u port, we'd now have a delta in a subsystem that is heavily modified in mainline and gets updates often.
- IIUC you write that this proposed patch is really a patch collection, containing 8266822 and 8267431 too. We normally downport patches 1:1.

It would be another story if Oracle had already downported it too, then we would have much a stronger incentive.

> We have run all the test cases under hotspot/jtreg/runtime/appcds includes these new cases in this patch,
and run some demos which has old classifies, such as spring-petclinic.

Sorry, I don't think this is enough by a long stretch. I'd need tier1-n tests across the board, across the whole range of supported platforms and architectures. We'd also like to see large scale tests with systems that use large amounts of pre-JDK6 classes. 

Bottomline, in my eye there is not a large enough benefit to justify the cost and risk involved. I think the workarounds we have are acceptable: users can just run JDK17 or should update their class libraries or just have to live without them in AppCDS.

Sorry, I know you put a lot of work in. When in doubt, before attempting a large backport, it is better to discuss it on the OpenJDK mailing lists to see what folks think.

Cheers, Thomas

-------------

PR: https://git.openjdk.java.net/jdk11u-dev/pull/856