Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge JITServer changes to CompilationThread.cpp/hpp #7454

Merged
merged 1 commit into from Oct 21, 2019

Conversation

mpirvu
Copy link
Contributor

@mpirvu mpirvu commented Oct 15, 2019

Signed-off-by: Marius Pirvu mpirvu@ca.ibm.com

@mpirvu mpirvu added the comp:jitserver Artifacts related to JIT-as-a-Service project label Oct 15, 2019
@mpirvu mpirvu added this to In progress in JIT as a Service via automation Oct 15, 2019
@mpirvu mpirvu force-pushed the JITServerChanges branch 2 times, most recently from 323178b to f09025a Compare October 15, 2019 19:42
@mpirvu
Copy link
Contributor Author

mpirvu commented Oct 15, 2019

Jenkins test sanity all jdk8,jdk11

@mpirvu mpirvu changed the title WIP: Merge JITServer changes to CompilationThread.cpp/hpp Merge JITServer changes to CompilationThread.cpp/hpp Oct 16, 2019
@mpirvu
Copy link
Contributor Author

mpirvu commented Oct 16, 2019

Jenkins test sanity plinux jdk8,jdk11

@mpirvu mpirvu requested a review from ymanton October 16, 2019 14:36
@mpirvu
Copy link
Contributor Author

mpirvu commented Oct 16, 2019

@ymanton This is a relatively difficult PR to review because I did a little bit of restructuring. The vast majority of the changes are in CompilationThread.cpp though.

@mpirvu
Copy link
Contributor Author

mpirvu commented Oct 16, 2019

Jenkins test sanity aix jdk8,jdk11

Copy link
Member

@ymanton ymanton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of it looks OK. My concentration started to fail in CompilationThread.cpp, but I'll try to give it one more pass later.

runtime/compiler/control/CompilationThread.cpp Outdated Show resolved Hide resolved
runtime/compiler/control/CompilationThread.cpp Outdated Show resolved Hide resolved
runtime/compiler/control/CompilationThread.cpp Outdated Show resolved Hide resolved
runtime/compiler/control/CompilationThread.cpp Outdated Show resolved Hide resolved
@mpirvu
Copy link
Contributor Author

mpirvu commented Oct 16, 2019

@ymanton I addressed all the review comments and pushed the new version.

Copy link
Member

@ymanton ymanton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@ymanton
Copy link
Member

ymanton commented Oct 17, 2019

Jenkins test sanity.functional all jdk8,jdk11

@mpirvu
Copy link
Contributor Author

mpirvu commented Oct 17, 2019

11:54:48  Compiling 4 files for BUILD_JIGSAW_TOOLS
11:54:48  Assertion failed at /home/jenkins/workspace/Build_JDK11_ppc64le_linux_Personal/build/linux-ppc64le-normal-server-release/vm/compiler/../compiler/control/CompilationThread.cpp:9789: comp
11:54:48  	comp object must always exist when calling compilationEnd

So, that test was not there for nothing. I wonder under what conditions we hit this.

@mpirvu
Copy link
Contributor Author

mpirvu commented Oct 17, 2019

So, looking at the code the JIT will call compilationEnd() to fail compilations it has no intent to compile. For instance, when purging the compilation queue because the JVM is shutting down, when all compilation threads are suspended or when we don't want to compile anymore (e.g. virtual address space is running low).

@ymanton
Copy link
Member

ymanton commented Oct 17, 2019

It looks like the existing compilationEnd() checks for comp != NULL in some places to deal with this situation, but not all of the code is guarded. Its hard to tell if the unguarded code will really execute if we enter without a comp, but if it does execute the early check & return from the jitaas branch are going change the behaviour.

@mpirvu
Copy link
Contributor Author

mpirvu commented Oct 17, 2019

I updated the code to allow for the possibility of comp object being null when compilation was aborted/failed.

@ymanton
Copy link
Member

ymanton commented Oct 17, 2019

Jenkins test sanity.functional all jdk8,jdk11

@mpirvu mpirvu force-pushed the JITServerChanges branch 2 times, most recently from 96fb171 to 34e956e Compare October 17, 2019 23:54
@mpirvu
Copy link
Contributor Author

mpirvu commented Oct 18, 2019

Jenkins test sanity.functional all jdk8,jdk11

@mpirvu
Copy link
Contributor Author

mpirvu commented Oct 18, 2019

Testing has passed.

@mpirvu
Copy link
Contributor Author

mpirvu commented Oct 18, 2019

More issues discovered in the jitaas branch. The entry can also be NULL in compilationEnd()

Signed-off-by: Marius Pirvu <mpirvu@ca.ibm.com>
@mpirvu
Copy link
Contributor Author

mpirvu commented Oct 18, 2019

Jenkins test sanity all jdk8,jdk11

@mpirvu
Copy link
Contributor Author

mpirvu commented Oct 19, 2019

Jenkins test sanity zlinux jdk11

@mpirvu
Copy link
Contributor Author

mpirvu commented Oct 19, 2019

Jenkins test sanity win jdk8

@mpirvu
Copy link
Contributor Author

mpirvu commented Oct 21, 2019

Tests passed on all platforms except win64 which has some infra problem:

13:27:13  Testing: Test option -Xjit:count=0,disableZ13
13:27:13  Test start time: 2019/10/19 12:27:12 Central Standard Time
13:27:13  Running command: "C:/Users/jenkins/workspace/Test_openjdk8_j9_sanity.functional_x86-64_windows_Personal/openjdkbinary/j2sdk-image\\bin\\java" -Xcompressedrefs  -Xdump -Xjit:count=0,disableZ13 -version
13:27:13  Time spent starting: 0 milliseconds
13:30:25  ***[TEST INFO 2019/10/19 12:30:12] ProcessKiller detected a timeout after 180000 milliseconds!***
13:30:25  INFO: getUnixPID() has failed indicating this is not a UNIX System.'Debug on timeout' is currently only supported on Linux.
13:30:25  Output from test:
13:30:25   [ERR] openjdk version "1.8.0_232-internal"
13:30:25   [ERR] OpenJDK Runtime Environment (build 1.8.0_232-internal-jenkins_2019_10_19_11_35-b00)
13:30:25   [ERR] Eclipse OpenJ9 VM (build HEAD-ebd657f54, JRE 1.8.0 Windows Server 2012 R2 amd64-64-Bit Compressed References 20191019_348 (JIT enabled, AOT enabled)
13:30:25   [ERR] OpenJ9   - ebd657f54
13:30:25   [ERR] OMR      - 148a6b09
13:30:25   [ERR] JCL      - 7eb1dfe231f based on jdk8u232-b09)
Cancelling nested steps due to timeout
23:18:30  Sending interrupt signal to process
23:18:50  After 20s process did not stop
23:18:50  jenkins.util.io.CompositeIOException: Unable to delete 'C:\Users\jenkins\workspace\Test_openjdk8_j9_sanity.functional_x86-64_windows_Personal@tmp\durable-e7bce985'. Tried 3 times (of a maximum of 3) waiting 0.1 sec between attempts.
23:18:50  	at jenkins.util.io.PathRemover.forceRemoveRecursive(PathRemover.java:99)
23:18:50  	at hudson.Util.deleteRecursive(Util.java:293)
23:18:50  	at hudson.FilePath$DeleteRecursive.invoke(FilePath.java:1274)

Copy link
Member

@ymanton ymanton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@ymanton ymanton merged commit 22cfe03 into eclipse-openj9:master Oct 21, 2019
JIT as a Service automation moved this from In progress to Done Oct 21, 2019
@mpirvu mpirvu deleted the JITServerChanges branch June 4, 2021 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:jitserver Artifacts related to JIT-as-a-Service project
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants