[memory_calculator] add a flag to ensure that -Xms=<-Xmx value> is set

[uqix]

No description provided.

[dmikusa]

That will be the default behavior.

If you override -Xmx, then you must also manually set -Xms if you want that behavior. See JAVA_TOOL_OPTIONS.

[uqix]

We don't specify -Xmx in JAVA_TOOL_OPTIONS so the calculated value is used, what we want is -Xms set to the same value as -Xmx by memory_calculator.

Glad to hear that will be the default behavior.

[uqix]

Maybe we can apply the same to -XX:MaxMetaspaceSize/-XX:MetaspaceSize?

[dmikusa]

Sorry, I misspoke there. It does not set the -Xms value. By default you get the following:

-XX:MaxDirectMemorySize=10M -Xmx652360K -XX:MaxMetaspaceSize=89015K -XX:ReservedCodeCacheSize=240M -Xss1M

1. Direct memory is always 10M
2. MaxMetaSpaceSize is calculated based on the number of class files present (0.35 * number of classes)
3. Reserved code cache defaults to 240M
4. Thread stack is 1M.
5. Heap gets what's left. i.e. container memory limit minus items 1-4 above.

Having said that, right now if you want to set the min & max values to be the same you would need to manually set the min values in JAVA_TOOL_OPTIONS. There isn't an easy way to get the maximum value dynamically. You'd have to run your app, see what values are generated & restart the app with the minimums set.

I switched this over to enhancement. I don't think we'd change the default unless there is sufficient evidence provided that this would be a benefit to all users. I know setting min to max is an old trick to reduce GC on startup, but I'm not sure how relevant this is with modern hardware, JVMs, and GC implementations. I'd want to see some benchmarking and GC logs to prove it's a tangible benefit before changing defaults.

That said, I'm not opposed to an option that would enable setting min to max. Like BP_MEMORY_CALC_INCLUDE_MINS or something like that. I can't commit to a timeframe for adding this, it would likely be a low priority feature given you can set the values manually. If you would be interested in submitting a PR, I could prioritize reviewing and merging that.

A PR would need to at least:

1. Add a conditional env variable to trigger the behavior (include in README & buildpack.toml)
2. Add the min settings if the env variable is set for heap and metaspace.
3. Add corresponding tests.
4. Allow a user to override the values.

[yaba]

@dmikusa The default values don't work out of the box on Azure App Services (S1 plan with 1.75Gb).
It was working before, what was the change? (I've used the Java sample provided). Thank you.

[dmikusa]

@yaba Please open a new issue and provide more details about the problem you're seeing. The full output of building your application would be helpful, as well as full output from starting the application up.