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

[Quarkus-CI] JVMCI Compiler does not support selected GC: concurrent mark sweep gc #2782

Closed
zakkak opened this issue Aug 24, 2020 · 11 comments · Fixed by #2797
Closed

[Quarkus-CI] JVMCI Compiler does not support selected GC: concurrent mark sweep gc #2782

zakkak opened this issue Aug 24, 2020 · 11 comments · Fixed by #2797
Assignees
Labels

Comments

@zakkak
Copy link
Collaborator

zakkak commented Aug 24, 2020

Describe the issue
#2143 now appears in graal's new quarkus workflow.
See https://github.com/oracle/graal/runs/1019872072?check_suite_focus=true#step:17:295

Looking for UseConcMarkSweepGC in the quarkus repository we see that the three elasticsearch tests use it.

In graal's quarkus workflow we use GraalVM as the JAVA_HOME

export JAVA_HOME=${GITHUB_WORKSPACE}/graaljdk

thus breaking those elasticsearch tests.

Elasticsearch seems to support G1GC for java > 11, but Quarkus has not abandoned java 8 support yet so it appears like it can't do the switch yet.

@dougxc
Copy link
Member

dougxc commented Aug 25, 2020

It's not clear what solution is you're looking for. We don't plan on adding support for the CMS GC to Graal.

@zakkak
Copy link
Collaborator Author

zakkak commented Aug 25, 2020

Hi @dougxc , not sure what's the best way to solve this.

I see the following paths:

  1. Since Graal guides suggest using GraalVM only for the native-image part (by setting GRAALVM_HOME to its path), we could point JAVA_HOME to an OpenJDK instance instead of GraalVM in the workflow. This would resolve the issue since CMS would work fine. However, this way we will only be testing graal's native-image with quarkus.
  2. Quarkus could consider dropping UseConcMarkSweepGC from the test's config.
  3. We could skip this test from graal's workflow

@Sanne
Copy link
Contributor

Sanne commented Aug 25, 2020

@zakkak I think it's ok to not enforce using UseConcMarkSweepGC: that is just used during integration testing, so to start an Elasticsearch cluster to connect to . We don't actually need to run Elasticsearch within the same JVM as Quarkus, and we also don't really need it to use the mark-sweep collector.

@Sanne
Copy link
Contributor

Sanne commented Aug 25, 2020

cc/ @yrodiere

@yrodiere
Copy link

We don't actually need to run Elasticsearch within the same JVM as Quarkus, and we also don't really need it to use the mark-sweep collector.

If what you mean is that we can use different Java binaries for Elasticsearch than for Quarkus, then yes, totally. That's what we do in Hibernate Search integration tests: Elasticsearch always runs on Java 11, while we change the JVM used for our integration tests to test various Java versions.

However... The Quarkus build uses a separate maven plugin to start Elasticsearch, but only for historical reasons. Nowadays, Elastic provides decent container images of Elasticsearch. They're not necessarily configured optimally for integration tests, but they will definitely be enough.

If you have to change your build, I'd recommend switching to a container-based approach to start Elasticsearch? Since you already use containers for other external services (postgresql, ...), you already have most of the build configuration ready. And that will get rid of these problems we keep having with the JVM used in Elasticsearch.

If you give me the green light, I could have a look this afternoon.

@zakkak
Copy link
Collaborator Author

zakkak commented Aug 25, 2020

Thanks @Sanne and @yrodiere.

The docker image sounds like a good approach to me.
Note that this should be done in Quarkus first, and then get integrated to Graal's workflow, since we try to keep graal's workflow as close to Quarkus' as possible (to ease syncing and avoid false negatives).

@Sanne
Copy link
Contributor

Sanne commented Aug 25, 2020

@yrodiere sure, feel free to change to containers if you prefer. Or just change the jvm.options files in our tests, as you prefer.

@zakkak Do you need us to fix Quarkus first or should we close this issue? I wouldn't classify it as a graalvm bug but I'm not sure of your process.

@zakkak
Copy link
Collaborator Author

zakkak commented Aug 25, 2020

@Sanne I agree, but since it affects GraalVM's CI I suggest we keep this issue open till we see green in the CI :)

@dougxc
Copy link
Member

dougxc commented Oct 30, 2020

Can we close this issue now @zakkak ?

@zakkak
Copy link
Collaborator Author

zakkak commented Nov 2, 2020

Hi @dougxc, merging #2797 should close this automatically.

@dougxc
Copy link
Member

dougxc commented Nov 2, 2020

@sanzinger can you please take care of #2797 soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants