eclipse-openj9 / openj9

Eclipse OpenJ9: A Java Virtual Machine for OpenJDK that's optimized for small footprint, fast start-up, and high throughput. Builds on Eclipse OMR (https://github.com/eclipse/omr) and combines with the Extensions for OpenJDK for OpenJ9 repo.
Other
3.28k stars 721 forks source link

ARM: Failures in ContendedFieldsTests_90_1 #1447

Open knn-k opened 6 years ago

knn-k commented 6 years ago

4 tests in ContendedFieldsTests_90_1 fail with openj9-openjdk9 for ARM as shown below:

FAILED: testContentionGroupsAndClass
java.lang.AssertionError: org.openj9.test.contendedfields.TestClasses$ContGroupContClass calculated size 64 actual size 24 difference -40
    at org.testng.AssertJUnit.fail(AssertJUnit.java:59)
    at org.testng.AssertJUnit.assertTrue(AssertJUnit.java:24)
    at org.openj9.test.contendedfields.FieldUtilities.checkObjectSize(FieldUtilities.java:67)
    at org.openj9.test.contendedfields.ContendedFieldsTests.testContentionGroupsAndClass(ContendedFieldsTests.java:194)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(java.base@9-internal/Native Method)

This is because the port library function j9sysinfo_get_cache_info() returns J9PORT_ERROR_SYSINFO_NOT_SUPPORTED (-355) on ARM.

smlambert commented 6 years ago

@DanHeidinga - any intention to add support for that function on ARM? If not, we should permanently exclude that test for ARM.

DanHeidinga commented 6 years ago

We'll need that function to be implemented if ARM is to be a first class platform.

I'm OK with excluding the test on ARM for now until someone is able to pick this up.

JamesKingdon commented 6 years ago

Out of curiosity, what uses that?

DanHeidinga commented 6 years ago

The @Contended annotation for laying out fields on their own cache line.

knn-k commented 6 years ago

I am OK with excluding the test on ARM for the time being.

JamesKingdon commented 6 years ago

I wonder how we should implement that. I guess we could build a database mapping from the cpu type info in /proc/cpuinfo to cache line-length, although it would be nice if there was something more direct. We do run slap into the problem of Big.little pairs with potentially different line lengths. If the result is only used for @Contended then the conservative answer is to return the larger value, but if it was used for e.g. flushing the cache lines then the smaller value would be more appropriate and no single answer will do.

smlambert commented 6 years ago

If all variations of the ContendedFieldsTests test fail, then adding <platformRequirements>^os.arm</platformRequirements> to the <test> in the playlist.xml file should exclude them for now https://github.com/eclipse/openj9/blob/master/test/Java8andUp/playlist.xml#L650 (would need to add an equivalent line for SE80 test case around L622, if this is also fails in Java8 on ARM).

DanHeidinga commented 6 years ago

@JamesKingdon we'd have to search the code base to find all callers and determine whether the conservative approach is sufficient or not.

As an aside, what's a Big.little pair?

JamesKingdon commented 6 years ago

Big.little is an architectural approach designed primarily for the mobile space. Instead of all the cores on the processor being the same some are more powerful than others - big cores and little cores. Typical configurations might use 2 large and 4 small cores, or 4+4. In early implementations only one set of cores would be powered up at a time, so you'd be running on the big cores for interactive use, but when the screen is off everything gets swapped to the small cores for background processing with minimum power use. At least with Linux it's now the norm to have all the cores powered up simultaneously and the scheduler decides which processes to run on which cores. For the most part, the intention is for the big and little cores to be functionally equivalent, so that other than performance there's no difference from a software perspective which core you land on. However, there was at least one example where the cores were sufficiently different that it mattered. I don't expect to see big.little used much in the server space, but for the hobby and embedded space it's likely to be quite common.

pdbain-ibm commented 6 years ago

The purpose of "@contended" is to prevent false sharing of memory locations, usually in a java.util.concurrent object. I spread out the object such that the lockword and visible fields of an object are on separate cache lines.

We could use a default (conservative) value for the cache line length in the field resolution code, since we already make conservative decisions for worst-case situations.

@DanHeidinga I can work on that if you wish.