search

cloudfoundry / java-buildpack-memory-calculator

Cloud Foundry JVM Memory Calculator
Apache License 2.0
642 stars 70 forks source link

Memory Calculator should prioritize last instance of command switch instead of first #12

Closed youngm closed 7 years ago

youngm commented 7 years ago

Tested this against the excellent JBP 4.3 release.

I would like to provide default memory configuration in my java_opts.yml and allow users to override that using the JAVA_OPTS app environment variable.

It appears Java takes the last of a duplicated parameter but the memory calculator appears to take the first.

java -Xss1M -Xss256k -XX:+PrintFlagsFinal | grep "intx ThreadStackSize"
     intx ThreadStackSize                          := 256                                 {pd product}

It appears in the example below that memory calculator prefers the first value. JBP appears to correctly place the environment JAVA_OPTS at the end of the start command. However, the memory calculator will think the application can use more heap than it really can.

cf set-env testapp JBP_CONFIG_JAVA_OPTS '{ java_opts: "-Xss256k" }'
cf set-env testapp JAVA_OPTS "-Xss1M"
nebhale commented 7 years ago

Yep, I was just wondering how this worked earlier this week. Seems reasonable.

glyn commented 7 years ago

Thanks for spotting this @youngm! Fixed in 80300d10fa1.

nebhale commented 7 years ago

@youngm Released as v3.9.0.RELEASE. When you get a moment, please give it a go for verification.

youngm commented 7 years ago

@nebhale gave it a quick test. It appears to work great now. Thanks for the fast turn around!

nebhale commented 7 years ago

Probably going out in a buildpack release either Thursday or Monday.