• Type: Bug
• Component: hotspot
• Sub-Component: compiler
• Affected Version: 21,22,23,24

• Priority: P3
• Status: Open
• Resolution: Unresolved
• OS: generic
• CPU: generic

• Submitted: 2023-11-09
• Updated: 2024-07-17

JDK 24
24Unresolved

I'm unclear on that as well. Maybe [~vlivanov] also knows more about it? 11-12-2023 Sorry I'm still unclear on exactly what the CI code is trying to determine. Is it asking if the given classloader is a known initiating loader for the target class? Or does it want to know if loading through the given classloader would actually resolve to the class already loaded by the boot loader? Or ??? 11-12-2023 I agree that CI code should not do any loading. But in this case, Integer has already been loaded by the bootstrap class loader without any protection domain. Shouldn't the CI code somehow be able to check these classes as well? Or am I missing something? I think if we want to keep the CI code like that, then we do not need to adapt the test because it relies on C2 not being able to inline bar() in this example. 08-12-2023 SystemDictionary::find_instance_klass() only checks the current class_loader, and doesn't delegate to the system class loader, so the application class loader hasn't initiated loading "Integer" yet. The CI code doesn't want to actually do any loading, it only wants to check if it's been done and only checks current class loader, which sort of makes sense because you could have a protection domain active for initiating loading for a non-system class loader. I think this is the case. It's probably less confusing in this CR to say "not loaded" rather than "unloaded". You could probably fix the test also by declaring a static member of type Integer in class Test. 04-12-2023 Yeah that sounds very confusing to me. A class is either loaded or not loaded. If loaded it has one defining loader and any number of initiating loaders. If any given classloader needs a particular class X then it would normally delegate to its parent loaders to find X and if not found then locate the class bytes itself and then define the class. The latter can't happen for a system class like Integer as it can only be loaded and defined by the bootloader. I would have said that find_instance_klass is checking if the given loader is the defining loader of the class, but in that case the application class loader would only contain Test, not String or Object! So I'm left unclear what question ciEnv::get_klass_by_name_impl() is actually trying to answer and why it selects the mechanism it does to try and answer it. ??? [~coleenp] can you shed any light here? 03-12-2023 Okay, I had to dig deeper to get a better understanding of why C2 cannot inline bar() in the first place in the following example when running with -XX:CompileOnly=Test::test: 995 81 b Test::test (5 bytes) @ 0 Test::bar (2 bytes) failed to inline: unloaded signature classes public class Test { public static void main(String[] args) { for (int i = 0; i < 10000; i++) { test(); } } public static void test() { bar(); // Not compiled before separately } private static Integer bar() { return null; } } When doing type flow analysis in C2, we create a ciSignature object for bar(). The constructor walks through the signature and calls ciEnv::get_klass_by_name_impl() for Integer which eventually calls SystemDictionary::find_instance_klass() with the class loader of Test, which is the application class loader. In find_instance_klass(), we get the dictionary of the application class loader which only includes Test, Object, and String since we've only walked through the public methods of Test to find main(). We call findMainMethod() which eventually calls getDeclaredMethods0() which adds the classes of the signatures of public methods to the dictionary in the VM. Thus, SystemDictionary::find_instance_klass() does not find Integer in the dictionary of the application class loader and returns null - does not matter what protection domain we use. As a result, we create a ciInstanceKlass object for Integer that is unloaded. Later in parsing, when trying to inline bar(), we fail to do so because the ciSignature object contains an unloaded class (i.e. Integer). This seems kinda odd to me. Maybe I'm missing something but I don't understand why ciEnv::get_klass_by_name_impl() is not also checking if the class was loaded with the platform or the bootstrap class loader but only with the class loader of the accessing klass. Is that an unnecessary limitation? Anyway, for TestInliningProtectionDomain, we use the fact that C2 cannot inline bar(). In compiler replay, we just load all the instance klasses dumped to the replay file with the same class loader. Therefore, ciEnv::get_klass_by_name_impl() can also find Integer when creating the ciSignature object of bar() because the dictionary contains all the instance klasses. Inlinining would succeed but it is prevented in order to force compiler replay to apply the same inlining decisions as in the normal run. This is tested with TestInliningProtectionDomain. 01-12-2023 I don't understand how any of the core classes loaded during VM initialization relate to "the protection domain of ProtectionDomainTestNoOtherCompilationPrivate" but they would all seem to be the same in that regard. Sorry but there is an aspect to this test that I am completely missing. 27-11-2023 That' right but even though Integer is loaded, it is unloaded for the protection domain of ProtectionDomainTestNoOtherCompilationPrivate. When C2 tries to inline the private method ProtectionDomainTestNoOtherCompilationPrivate::bar(), it fails with "failed to inline: unloaded signature classes". However, when doing compiler replay, we wrongly inlined bar() because we loaded all classes without an explicit protection domain. JDK-8275868 fixed that such that compiler replay used the correct protection domain to load classes and also not inline bar(). To test this, we just need to have a Java API class that is always loaded without explicit protection domain on VM init - we picked Integer which might not be the best choice. So, we could use Class instead. 24-11-2023 I'm still perplexed by the expectations of this test around class resolution. I thought the test wanted a class that had not been resolved hence Integer. The Integer class is loaded during VM init. Running `java -version` loads and initializes 273 classes. 24-11-2023 I think the test just wants to use a Java API class that is always loaded on VM startup. Which class could we use for that apart from Object and String? Maybe class Class? 23-11-2023 Okay, that's good. I will adapt the test to use Class to get rid of the Integer dependency. 23-11-2023 Either that or System, or ClassLoader. 23-11-2023 Okay, thanks for the update! 17-11-2023 The test should be corrected but not necessarily for jdk 22. 17-11-2023 [~jlaskey] I've seen that you reinstated TestInliningProtectionDomain.java in the review for JDK-8315458 in a later commit here: https://github.com/openjdk/jdk/pull/16461/commits/dd6703e7ca08c2cf81021b40b4dab1c2c450d5a2 Does this mean that this test does not need to be addressed/fixed? 17-11-2023 If this is not fixed in a timely manner, feel free to problemlist this test with JDK-8315458 to avoid noise in the CI. 10-11-2023 ILW = Test failure due to previously unloaded class that is now loaded with JDK-8315458, only with JDK-8315458, no workaround = MLH = P4 10-11-2023 [~jlaskey] If you want this test fixed by hotspot team it should be re-assigned to compiler team. Thanks. 10-11-2023 [~jlaskey] Thanks for clarifying, yes is this a rather subtle aspect of the test and not one based on any specification AFAICS. 10-11-2023 [~dholmes] This test relies on the launcher to resolve or not resolve Integer. That dependency has to go away. class ProtectionDomainTestNoOtherCompilationPrivate { public static void main(String[] args) { for (int i = 0; i < 10000; i++) { test(); } } public static void test() { bar(); // Not compiled before separately } // Integer should be unresolved for the protection domain of this class even though getDeclaredMethods is called in normal // run when validating main() method. In this process, only public methods of this class are visited and its signature // classes are resolved. Since this method is private, the signature classes are not resolved for this protection domain. // Inlining of bar() should fail in normal run with "unresolved signature classes". Therefore, replay compilation should // also not inline bar(). private static Integer bar() { InliningFoo.foo(); return null; } } 10-11-2023 [~jlaskey] can you elaborate please. I don't see anything in this test that isn't the same as a hundred other tests. 10-11-2023