RFR: 8081669: aarch64: JTreg TestStable tests failing
Hi, The following webrev http://cr.openjdk.java.net/~enevill/8081669/webrev.00/ fixes a number of TestStable tests. This patch was contributed by alexander.alexeev@caviumnetworks.com The following are the test failures that are fixed by this patch compiler/stable/TestStableByte.java compiler/stable/TestStableBoolean.java compiler/stable/TestStableChar.java compiler/stable/TestStableFloat.java compiler/stable/TestStableObject.java compiler/stable/TestStableDouble.java compiler/stable/TestStableInt.java compiler/stable/TestStableLong.java compiler/stable/TestStableShort.java The problem is that the method 'get' in StableConfiguration is supposed to return true if the method is server compiled, false otherwise. On aarch64 it is always returning true, even when the method is client compiled. The reason for this is that aarch64 differs from other ports in that it always deopts on patching. This means that the method 'get' deopts immediately when compiled with -Xcomp because it hits an unresolved method call. This means that the method is now executing in the interpreter. When the method 'get' is executing in the interpreter, it uses the value of java.vm.name to determine whether the method would be server compiled if it were to be compiled. This ends up returning true on aarch64, because it is a server compiler. However in the case where we force it not to server compile by using -XX:+TieredCompilation and -XX:TieredStopAtLevel=1 (as in the TestStable tests) this will be incorrect. The solution is to introduce a simple (null) method 'get1' which will never be deopted (because there is never anything to patch) and uses this as the basis for deciding whether we are server compiling or not. This is more fully explained in the inline comment in the patch. Please review and if appropriate I will push. All the best, Ed.
On 06/02/2015 10:26 AM, Edward Nevill wrote:
Please review and if appropriate I will push.
That sounds correct to me, but I need a JDK9 reviewer. Andrew.
Looks good. Best regards, Vladimir Ivanov On 6/2/15 12:26 PM, Edward Nevill wrote:
Hi,
The following webrev
http://cr.openjdk.java.net/~enevill/8081669/webrev.00/
fixes a number of TestStable tests.
This patch was contributed by alexander.alexeev@caviumnetworks.com
The following are the test failures that are fixed by this patch
compiler/stable/TestStableByte.java compiler/stable/TestStableBoolean.java compiler/stable/TestStableChar.java compiler/stable/TestStableFloat.java compiler/stable/TestStableObject.java compiler/stable/TestStableDouble.java compiler/stable/TestStableInt.java compiler/stable/TestStableLong.java compiler/stable/TestStableShort.java
The problem is that the method 'get' in StableConfiguration is supposed to return true if the method is server compiled, false otherwise.
On aarch64 it is always returning true, even when the method is client compiled. The reason for this is that aarch64 differs from other ports in that it always deopts on patching.
This means that the method 'get' deopts immediately when compiled with -Xcomp because it hits an unresolved method call. This means that the method is now executing in the interpreter.
When the method 'get' is executing in the interpreter, it uses the value of java.vm.name to determine whether the method would be server compiled if it were to be compiled. This ends up returning true on aarch64, because it is a server compiler.
However in the case where we force it not to server compile by using -XX:+TieredCompilation and -XX:TieredStopAtLevel=1 (as in the TestStable tests) this will be incorrect.
The solution is to introduce a simple (null) method 'get1' which will never be deopted (because there is never anything to patch) and uses this as the basis for deciding whether we are server compiling or not.
This is more fully explained in the inline comment in the patch.
Please review and if appropriate I will push.
All the best, Ed.
The only concern I have is that I don't see Alexander on OCA list [1]. In order to proceed with the fix, he should sign OCA first. Best regards, Vladimir Ivanov [1] http://www.oracle.com/technetwork/community/oca-486395.html On 6/2/15 5:17 PM, Vladimir Ivanov wrote:
Looks good.
Best regards, Vladimir Ivanov
On 6/2/15 12:26 PM, Edward Nevill wrote:
Hi,
The following webrev
http://cr.openjdk.java.net/~enevill/8081669/webrev.00/
fixes a number of TestStable tests.
This patch was contributed by alexander.alexeev@caviumnetworks.com
The following are the test failures that are fixed by this patch
compiler/stable/TestStableByte.java compiler/stable/TestStableBoolean.java compiler/stable/TestStableChar.java compiler/stable/TestStableFloat.java compiler/stable/TestStableObject.java compiler/stable/TestStableDouble.java compiler/stable/TestStableInt.java compiler/stable/TestStableLong.java compiler/stable/TestStableShort.java
The problem is that the method 'get' in StableConfiguration is supposed to return true if the method is server compiled, false otherwise.
On aarch64 it is always returning true, even when the method is client compiled. The reason for this is that aarch64 differs from other ports in that it always deopts on patching.
This means that the method 'get' deopts immediately when compiled with -Xcomp because it hits an unresolved method call. This means that the method is now executing in the interpreter.
When the method 'get' is executing in the interpreter, it uses the value of java.vm.name to determine whether the method would be server compiled if it were to be compiled. This ends up returning true on aarch64, because it is a server compiler.
However in the case where we force it not to server compile by using -XX:+TieredCompilation and -XX:TieredStopAtLevel=1 (as in the TestStable tests) this will be incorrect.
The solution is to introduce a simple (null) method 'get1' which will never be deopted (because there is never anything to patch) and uses this as the basis for deciding whether we are server compiling or not.
This is more fully explained in the inline comment in the patch.
Please review and if appropriate I will push.
All the best, Ed.
Vladimir, I am contributing on behalf of Cavium Inc. It actually exists in the list. Is it enough? Regards, Alexander -----Original Message----- From: aarch64-port-dev [mailto:aarch64-port-dev-bounces@openjdk.java.net] On Behalf Of Vladimir Ivanov Sent: Tuesday, June 2, 2015 7:51 PM To: edward.nevill@linaro.org; aarch64-port-dev@openjdk.java.net; hotspot-dev Source Developers Subject: Re: [aarch64-port-dev ] RFR: 8081669: aarch64: JTreg TestStable tests failing The only concern I have is that I don't see Alexander on OCA list [1]. In order to proceed with the fix, he should sign OCA first. Best regards, Vladimir Ivanov [1] http://www.oracle.com/technetwork/community/oca-486395.html On 6/2/15 5:17 PM, Vladimir Ivanov wrote:
Looks good.
Best regards, Vladimir Ivanov
On 6/2/15 12:26 PM, Edward Nevill wrote:
Hi,
The following webrev
http://cr.openjdk.java.net/~enevill/8081669/webrev.00/
fixes a number of TestStable tests.
This patch was contributed by alexander.alexeev@caviumnetworks.com
The following are the test failures that are fixed by this patch
compiler/stable/TestStableByte.java compiler/stable/TestStableBoolean.java compiler/stable/TestStableChar.java compiler/stable/TestStableFloat.java compiler/stable/TestStableObject.java compiler/stable/TestStableDouble.java compiler/stable/TestStableInt.java compiler/stable/TestStableLong.java compiler/stable/TestStableShort.java
The problem is that the method 'get' in StableConfiguration is supposed to return true if the method is server compiled, false otherwise.
On aarch64 it is always returning true, even when the method is client compiled. The reason for this is that aarch64 differs from other ports in that it always deopts on patching.
This means that the method 'get' deopts immediately when compiled with -Xcomp because it hits an unresolved method call. This means that the method is now executing in the interpreter.
When the method 'get' is executing in the interpreter, it uses the value of java.vm.name to determine whether the method would be server compiled if it were to be compiled. This ends up returning true on aarch64, because it is a server compiler.
However in the case where we force it not to server compile by using -XX:+TieredCompilation and -XX:TieredStopAtLevel=1 (as in the TestStable tests) this will be incorrect.
The solution is to introduce a simple (null) method 'get1' which will never be deopted (because there is never anything to patch) and uses this as the basis for deciding whether we are server compiling or not.
This is more fully explained in the inline comment in the patch.
Please review and if appropriate I will push.
All the best, Ed.
Hi Vladimir, - Alexander is employed as a contractor by Cavium - Cavium have signed the OCA - Alexander is using his Cavium email address I have checked this with Dalibor Topic (on cc) and my understanding was that this was sufficient to allow Alexander to contribute. All the best, Ed. On Tue, 2015-06-02 at 19:50 +0300, Vladimir Ivanov wrote:
The only concern I have is that I don't see Alexander on OCA list [1]. In order to proceed with the fix, he should sign OCA first.
Best regards, Vladimir Ivanov
[1] http://www.oracle.com/technetwork/community/oca-486395.html
On 6/2/15 5:17 PM, Vladimir Ivanov wrote:
Looks good.
Best regards, Vladimir Ivanov
On 6/2/15 12:26 PM, Edward Nevill wrote:
Hi,
The following webrev
http://cr.openjdk.java.net/~enevill/8081669/webrev.00/
fixes a number of TestStable tests.
This patch was contributed by alexander.alexeev@caviumnetworks.com
The following are the test failures that are fixed by this patch
compiler/stable/TestStableByte.java compiler/stable/TestStableBoolean.java compiler/stable/TestStableChar.java compiler/stable/TestStableFloat.java compiler/stable/TestStableObject.java compiler/stable/TestStableDouble.java compiler/stable/TestStableInt.java compiler/stable/TestStableLong.java compiler/stable/TestStableShort.java
The problem is that the method 'get' in StableConfiguration is supposed to return true if the method is server compiled, false otherwise.
On aarch64 it is always returning true, even when the method is client compiled. The reason for this is that aarch64 differs from other ports in that it always deopts on patching.
This means that the method 'get' deopts immediately when compiled with -Xcomp because it hits an unresolved method call. This means that the method is now executing in the interpreter.
When the method 'get' is executing in the interpreter, it uses the value of java.vm.name to determine whether the method would be server compiled if it were to be compiled. This ends up returning true on aarch64, because it is a server compiler.
However in the case where we force it not to server compile by using -XX:+TieredCompilation and -XX:TieredStopAtLevel=1 (as in the TestStable tests) this will be incorrect.
The solution is to introduce a simple (null) method 'get1' which will never be deopted (because there is never anything to patch) and uses this as the basis for deciding whether we are server compiling or not.
This is more fully explained in the inline comment in the patch.
Please review and if appropriate I will push.
All the best, Ed.
Edward, Alexander, thanks for the clarifications! My bad, missed Cavium in the OCA list. Best regards, Vladimir Ivanov On 6/2/15 8:11 PM, Edward Nevill wrote:
Hi Vladimir,
- Alexander is employed as a contractor by Cavium - Cavium have signed the OCA - Alexander is using his Cavium email address
I have checked this with Dalibor Topic (on cc) and my understanding was that this was sufficient to allow Alexander to contribute.
All the best, Ed.
On Tue, 2015-06-02 at 19:50 +0300, Vladimir Ivanov wrote:
The only concern I have is that I don't see Alexander on OCA list [1]. In order to proceed with the fix, he should sign OCA first.
Best regards, Vladimir Ivanov
[1] http://www.oracle.com/technetwork/community/oca-486395.html
On 6/2/15 5:17 PM, Vladimir Ivanov wrote:
Looks good.
Best regards, Vladimir Ivanov
On 6/2/15 12:26 PM, Edward Nevill wrote:
Hi,
The following webrev
http://cr.openjdk.java.net/~enevill/8081669/webrev.00/
fixes a number of TestStable tests.
This patch was contributed by alexander.alexeev@caviumnetworks.com
The following are the test failures that are fixed by this patch
compiler/stable/TestStableByte.java compiler/stable/TestStableBoolean.java compiler/stable/TestStableChar.java compiler/stable/TestStableFloat.java compiler/stable/TestStableObject.java compiler/stable/TestStableDouble.java compiler/stable/TestStableInt.java compiler/stable/TestStableLong.java compiler/stable/TestStableShort.java
The problem is that the method 'get' in StableConfiguration is supposed to return true if the method is server compiled, false otherwise.
On aarch64 it is always returning true, even when the method is client compiled. The reason for this is that aarch64 differs from other ports in that it always deopts on patching.
This means that the method 'get' deopts immediately when compiled with -Xcomp because it hits an unresolved method call. This means that the method is now executing in the interpreter.
When the method 'get' is executing in the interpreter, it uses the value of java.vm.name to determine whether the method would be server compiled if it were to be compiled. This ends up returning true on aarch64, because it is a server compiler.
However in the case where we force it not to server compile by using -XX:+TieredCompilation and -XX:TieredStopAtLevel=1 (as in the TestStable tests) this will be incorrect.
The solution is to introduce a simple (null) method 'get1' which will never be deopted (because there is never anything to patch) and uses this as the basis for deciding whether we are server compiling or not.
This is more fully explained in the inline comment in the patch.
Please review and if appropriate I will push.
All the best, Ed.
On Tue, 2015-06-02 at 20:16 +0300, Vladimir Ivanov wrote:
Edward, Alexander, thanks for the clarifications!
My bad, missed Cavium in the OCA list.
Best regards, Vladimir Ivanov
NP, Thanks for the review. I have pushed the change, All the best, Ed.
participants (5)
-
Alexeev, Alexander
-
Andrew Haley
-
Edward Nevill
-
Edward Nevill
-
Vladimir Ivanov