CB-14139 android: Add jvmargs flag for custom values

[erisu]

Platforms affected

Android

What does this PR do?

Added an additional flag --jvmargs to allow users to customize the Gradle's JVM memory heap size settings.

Document updates have been submitted in cordova-docs PR 839.

JIRA Ticket: CB-14139

What testing has been done on this change?

• cordova build and run with and without flag settings.

Checklist

• Reported an issue in the JIRA database
• Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
• Added automated test coverage as appropriate for this change.

[raphinesse]

Couldn't we instead remove the default jvmargs setting and use one of the various other transparent methods of setting this? As in environment variables (per command) or Gradle properties.

I'm just asking. New features are hard to get rid of, once added.

[erisu]

@raphinesse

I do not mind if the org.gradle.jvmargs is defined in the gradle.properties file. I also agree that once a new feature is added, it will be hard to remove.

Gradle's Documentation also references the usage of gradle.properties for setting the org.gradle.jvmargs.

The only potential issue is for users that have already created their own gradle.properties file.

If the command-line flag is removed from GradleBuilder.js and StudioBuilder.js, users with the gradle.properties file may need to manually add the property.

Gradle's default maximum heap size is Xmx1024m while Cordova defines Xmx2048m.

The command-line flag setting was committed for enabling the dex process for large projects.

[raphinesse]

Thanks for your Analysis.

Looking at the issue this is supposed to fix, I wonder if removing the default memory setting is the right thing here. The way I see it, we would be improving support for 32 bit JVMs while worsening the situation for large projects. That does not sound like a good deal to me.

Is 32bit Java support even relevant? If so, IMHO the best thing would be detecting it and limit the heap size accordingly. However, we probably need the check_reqs cleanup that I put on the roadmap for next major first.

Does that make sense or am I getting something wrong? I would definitely like a third opinion on this.

Edit: Closed by accident. Sorry

[codecov-io]

Codecov Report

Merging #459 into master will decrease coverage by 0.03%.
The diff coverage is 0%.

@@           Coverage Diff            @@
##           master   #459      +/-   ##
========================================
- Coverage   52.03%    52%   -0.04%     
========================================
  Files          17     17              
  Lines        1695   1696       +1     
  Branches      312    313       +1     
========================================
  Hits          882    882              
- Misses        813    814       +1

Impacted Files	Coverage Δ
bin/templates/cordova/lib/build.js	12.94% <0%> (-0.1%)	⬇️
...in/templates/cordova/lib/builders/GradleBuilder.js	20.75% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 46a036e...85bfb24. Read the comment docs.

[janpio]

Huh @erisu? What happened?

[erisu]

@janpio, sorry for the missing comment.

The more I think about it, it's probably better to not add an additional flag to set the jvmargs. It is a quicker and simpler solution but as mentioned, it would be harder to remove the flag in the future.

Using the gradle.properties would be better in the long run.

Looking back at the potential issue mentioned above when using the properties file, I think we just need to create a GradlePropertiesConfigParser/Writer.

If the file doesn't exist, we can either

• Create the file with our defaults
• Continue using the inline command

If the file already exists, parse the file to see if they have the default variables (excluding its value). If the default arguments are missing:

• Edit the file with our defaults
• Continue using the inline command

If they already have the arguments define, then just accept their values.

Also, I feel the amount of 32bit users could probably be very low and this would be an edge case. Since the user had a temporary workaround, we might be able to focus on removing all of the command-line usage in-favor of the gradle.properties instead of just jvmargs.

The user's temporary workaround did mention setting the environment variables affect the whole system but, it might be possible to set the variable during execution. This shouldn't affect the whole system. _JAVA_OPTIONS=-Xmx512m cordova .... The only question is if the set variable is still set when gradle runs.