RFR 8143628: Fork sun.misc.Unsafe and jdk.internal.misc.Unsafe native method tables

Paul Sandoz paul.sandoz at oracle.com
Thu Dec 3 09:51:07 UTC 2015

> On 3 Dec 2015, at 03:58, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:
> I finally had a chance to look at this.  Why are there pages of commit messages in these webrevs?

Not sure, it seems to be a bug with webrev (and may be a consequence of the interaction with patch queues, but i have not had time to dig into why this occurs).

> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8143628-unsafe-native-hotspot/webrev/src/share/vm/prims/unsafe.cpp.udiff.html
> +static JNINativeMethod sun_misc_Unsafe_methods[] = {
> This looks like a copy of the list of methods that we already have.

It's currently a subset of jdk_internal_misc_Unsafe_methods, notice that the entries for the following are not present in sun_misc_Unsafe_methods:

  private native boolean unalignedAccess0();
  private native boolean isBigEndian0();

The internal one will further diverge later on as we include additional access methods, and may likely becoming intersecting sets. Hence i deliberately kept them separate.

> +static JNINativeMethod jdk_internal_misc_Unsafe_methods[] = {
> Can you just have:
> +static JNINativeMethod sun_misc_Unsafe_methods[] = methods; +static JNINativeMethod jdk_internal_misc_Unsafe_methods[] = methods; ???
> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8143628-unsafe-native-hotspot/webrev/test/runtime/Unsafe/JdkInternalMiscUnsafeAccessTestBoolean.java.html
> I don't see the benefit of testng here.

The benefits are:

  1) Each test is clearly identified
  2) Each test runs regardless of the failure of any other test
  3) Each test result is reported

> You probably already have Assert in our test library.
> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8143628-unsafe-native-hotspot/webrev/test/runtime/Unsafe/generate-unsafe-access-tests.sh.html
> Is this script and template how these tests were generated?

Yes, it’s the same pattern as used in nio tests. If we evolve the tests we will edit the template and run the generation script rather than potentially editing 18 files.

> It doesn't seem like it needs to be checked in (and is missing copyright).

Ah, thanks, i will add the copyright.

> Maybe since the tests are stressing the compilation characteristics of the unsafe API, these tests belong in the test/compiler/unsafe directory.  Maybe the compiler group doesn't mind slogging through testng to find bugs.

But how much of an actual slog is it? (see my reply to you on hotspot-dev)

If i cannot persuade you and others in the runtime team (my powers of persuasion are failing and it is looking unlikely, but i am still trying :-) ), i will move them to the idk test area.


More information about the hotspot-dev mailing list