search

playframework / twirl

Twirl is Play's default template engine
Apache License 2.0
549 stars 108 forks source link

[Gradle] Use `processIsolation` instead of `classLoaderIsolation` #746

Closed ru5j4r0 closed 6 months ago

ru5j4r0 commented 8 months ago

Hi. First of all, I appreciate your great work. This is my first PR for the Play projects, so sorry in advance if I'm doing something wrong.

I was trying to migrate a few Play Framework projects to Gradle, but I couldn't migrate some of them because compileTwirl task gave me an out of memory error of JVM's Metaspace. I was able to migrate a project that has dozens of views, but I couldn't which has around 1,000 views. My PC has 32GB RAM, but increasing Xmx nor MaxMetaspaceSize didn't fix the issue.

After searching for a bit, I found this Gradle's issue. I forked the Twirl repo, replaced classLoaderIsolation with processIsolation, published it to mavenLocal() and using that SNAPSHOT version of the Twirl plugin from my local Maven repo actually fixed the problem.

I just filed a PR right away because it's a small change. What are your thoughts on this change?

Possibly related

Other projects doing the same fix

ru5j4r0 commented 8 months ago

I suppose the drawback of using processIsolation is a increased overhead due to the need to start a new process? Perhaps I can add an option to choose the isolation method, in case replacing classLoaderIsolation with processIsolation by default is not acceptable.

ihostage commented 7 months ago

Hi, @ru5j4r0!

I suppose the drawback of using processIsolation is a increased overhead due to the need to start a new process?

It's okay, don't worry. I will investigate this issue in more detail next week.

ru5j4r0 commented 6 months ago

@ihostage I’m the one who owes you an apology for opening a PR without tests or reproducible examples. Thank you for all your work!