Set minimum heap size for native Turbine to 512 MB

[fmeum]

No description provided.

[fmeum]

@cushon I found a minimum heap size of 512 MB to give a very nice speedup on my local machine (can only test on Linux right now unfortunately). Do you happen to know how you ended up with the 3 GB minimum for Google? Do you think that the benchmark I am using is reasonable?

[cushon]

@cushon I found a minimum heap size of 512 MB to give a very nice speedup on my local machine (can only test on Linux right now unfortunately). Do you happen to know how you ended up with the 3 GB minimum for Google? Do you think that the benchmark I am using is reasonable?

This all looks reasonable to me.

I tuned it in early 2022, and at the time we were giving JavaBuilder 3g of heap, so I tried the same for turbine. I don't remember if I collected measurements for smaller values, and for Google builds I wasn't really focused on the local resource estimate.

The benchmark seems like a good measure of average performance. The heap sizing we're using for JavaBuilder at Google gets dragged up by a handful of pathological examples that include very large generated files, etc. I have a TODO to allow tuning jvm flags for JavaBuilder for individual targets, to allow setting smaller defaults and then tuning for those outlier targets. It's possible we'll eventually want to do that for turbine, and I noticed that it also supports runtime -Xmx and -Xms flags (vs. the build-time flags), which could allow tuning it without rebuilding the native image: https://www.graalvm.org/latest/reference-manual/native-image/optimizations-and-performance/MemoryManagement/#java-heap-size

[fmeum]

That's a good point, with 4a29f08#diff-5d2185ff662bce191b9992a9c9aaba4baadfe1216d453160afaab68b2151d85bL108
settings -Xmx should work for native Turbine. I will give that a try.

Edit: Doesn't work yet since the ability to set args is not exposed to the Starlark interface of java_runtime. So this seems to be the only way to bump the heap size that is available at the moment.

[fmeum]

@cushon As you said, I don't think that there is currently any other way than setting the min heap size at image build time. I won't make any further changes, this is ready for review.

[brentleyjones]

Should this be cherry-picked into 7.1?

[fmeum]

@brentleyjones This entirely lives in java_tools prebuilts, but I do plan to cherry-pick a rules_java update with this change into 7.1.0. Currently it's not even live at HEAD.

We can still cherry-pick this change for transparency.

[fmeum]

@bazel-io flag

[iancha1992]

@bazel-io fork 7.1.0

[fmeum]

@keertk If you have the time, could you create a new java_tools release with this change?

[keertk]

@fmeum any chance this can wait till early Jan? I'm out until then. If not, perhaps @hvadehra can help?

[fmeum]

@keertk No worries, this can wait!

[keertk]

Tracked here: bazelbuild/java_tools#87

[keertk]

Done -
java_tools: https://github.com/bazelbuild/java_tools/releases/tag/java_v13.4
rules_java: https://github.com/bazelbuild/rules_java/releases/tag/7.4.0