RFR (L) JDK-8154239: -Xbootclasspath/a breaks exploded build

[Lois Foltan]

On 7/22/2016 3:55 PM, Karen Kinnear wrote:
> Lois,
>
> This looks very cleanly done! Thank you.
Thank you for the review Karen!

>
> Couple of minor comments:
> 1) -Xpatch is used in a number of comments - since they are new and this overlaps with Harold’s change - not sure
> if you want to change to —module-patch or choose to leave alone. Your call.
Yes, that will be a merge issue if this change bubbles up from the hs 
repo and Harold's comes in via the jake repo into jdk9/dev.  We will 
monitor this and make the appropriate changes.

>
> 2) Could you please add a comment that the boot loader append path does not support named modules?
>      - possibly in classLoader.hpp?
Done.

>
> 3) Could you possibly add a comment about the rule for —module-patch? i.e. the way this is coded
> I believe the rule is that the patch can include a partial module, so that if you do not find a specific
> class in the patch entries you still look in the core entries. (Perhaps in ClassLoader.cpp under Load Attempt #1 ?)
Done.

>
> 4) did you need to write any new tests or change the expected behavior? Or were the existing tests sufficient, and needed
> to be run with the exploded module build?
There was ample tests in our existing test systems that appeared as test 
failures when running with the exploded module build.  I ran most tests 
twice, once with a java runtime image and again with an exploded module 
build.

Thanks,
Lois

>
> I do not need to see another web rev.
>
> thanks,
> Karen
>
>
>> On Jul 19, 2016, at 2:09 PM, Lois Foltan <lois.foltan at oracle.com> wrote:
>>
>> Hello,
>>
>> Please review the following fix:
>>
>> Webrev:
>>     http://cr.openjdk.java.net/~lfoltan/bug_jdk8154239/webrev/
>>
>> Bug: -Xbootclasspath/a breaks exploded build
>>     https://bugs.openjdk.java.net/browse/JDK-8154239
>>
>> Summary:
>> The JVM was incorrectly handling how to set up and search the ClassPathEntry's for directories representing the module definitions in an exploded module build situation for the boot class loader. The incorrect set up and search was particularly exposed when -Xbootclasspath/a was specified in conjunction with a exploded module build.  In an exploded module build, when Modules::define_modules was called to define a module to the boot loader, a ClassPathEntry was simply appended to the boot loader's search list.  At load class time, a class would then be searched by simply traversing that list without any regards to the module that class was defined in.  This is incorrect behavior.  The class should only be searched for in the location of the module's definition that it was defined within.  The fix now records each module and it's exploded build location in order to adhere to this rule.
>>
>> The other piece to this problem was that if -Xbootclasspath/a was specified, it was incorrectly injected prior to the majority of ClassPathEntry's for the exploded modules, yielding a broken and incorrect search order for the boot loader.  Introducing the concept of a "base" or "core" piece for the boot loader which is either the java runtime image or the exploded modules build and changing the ClassLoader::_first_append_entry to truly be only appended entries added via -Xbootclasspath/a or jvmti appended entries, introduces a clean way for how this information is stored in the ClassLoader data structure that maps directly to how ClassLoader::load_class() searches that information.
>>
>> 215   // The boot class path consists of 3 ordered pieces:
>> 216   //  1. the module/path pairs specified to -Xpatch
>> 217   //    -Xpatch:<module>=<file>(<pathsep><file>)*
>> 218   //  2. the base piece
>> 219   //    [jimage | build with exploded modules]
>> 220   //  3. boot loader append path
>> 221   //    [-Xbootclasspath/a]; [jvmti appended entries]
>> 222   //
>> 223   // The boot loader must obey this order when attempting
>> 224   // to load a class.
>>
>>
>> Testing:
>> Full hotspot nightly testing with a java runtime image build
>> JDK java/io, java/lang, java/util with both a java runtime image and an exploded module build
>> JCK lang & vm with both a java runtime image and an exploded module build
>> Hotspot jtreg tests with both a java runtime image and an exploded module build
>>