Openj9: String compression flag class loading event does not trigger virtual guard patching

[fjeremic]

In https://github.com/eclipse/omr/pull/2013 we introduced a sideEffectGuard for the class load of java/lang/String$StringCompressionFlag, however this guard does not trigger the associated virtual guard patch when the respective class is loaded. This can easily be observed by the following simple unit test:

class Test
{
    public static void main(String[] args)
    {
        foo();
        foo();
    }

    public static void foo()
    {
        String s = "1";
        System.out.println(s.length());
        s = "\u2202";
        System.out.println(s.length());
    }
}

Running under two modes (-Xint) and (-Xjit) produces different results:

> bin/java -XX:+StringCompression -Xint Test
1
1
1
1

> bin/java -XX:+StringCompression -Xjit Test
1
-2147483647
1
-2147483647

[fjeremic]

@yanluo7 could you please help to investigate this? I will be looking as well.

[yanluo7]

@fjeremic I will look into it. One thing I found is that when I ran java -version, there is a non-compressed String being created somewhere in the jcl and I suspect the string creation is done in the JVM natives in StringTable.cpp. The natives seems to set the compression flag field correctly but need to look into if it would correctly trigger the patching. Will look into it.

[JasonFengJ9]

Could it be related to java/lang/String.coder field not initialized, see https://github.com/eclipse/openj9/pull/782?

[andrewcraik]

The coder field is not part of this issue. We need to figure out long term how the coder field is going to work because it is giving us a size disadvantage - we store the same information in the sign bit of our count field. Ideally, non-String classes should not be reaching in and accessing that field - there should be a getter method so we can have a divergent internal representation that achieves the same behavior.

[fjeremic]

Ideally, non-String classes should not be reaching in and accessing that field - there should be a getter method so we can have a divergent internal representation that achieves the same behavior.

Agreed. Such a thing already exists here:

https://github.com/eclipse/openj9/blob/02fdf14c71fb83f76deaa4f264b5c6748afc820b/jcl/src/java.base/share/classes/java/lang/String.java#L87-L93

Although I think I've encountered at least one place in OpenJDK where they reach into String via reflection and extract the field. It should instead be calling this getter.

[JasonFengJ9]

An issue https://github.com/ibmruntimes/openj9-openjdk-jdk9/issues/91 has been opened to request a JCL change to remove this field in the future release.

there is a non-compressed String being created somewhere in the jcl and I suspect the string creation is done in the JVM natives in StringTable.cpp.

@yanluo7 was this observed in Java 9 or 10, if it is Java 10, there is a problem in coder initialization.

[fjeremic]

@yanluo7 was this observed in Java 9 or 10, if it is Java 10, there is a problem in coder initialization.

This was observed on Java 8.

[JasonFengJ9]

@fjeremic thanks for clarification, the problem can't be related to java/lang/String.coder then.

[fjeremic]

@smlambert I'd like to contribute the unit test in the issue description so it runs on our PR Jenkins tests. How might I go about doing that? This test would need to run with a JVM invoked with -XX:+StringCompression. Could you point me to any documentation?

[smlambert]

I'll add details to #788 shortly for how to go about adding the test. Thanks for the question! 👍