Set a global -Xmx default for JVMs

Review Request #3705 — Created April 15, 2016 and submitted

benjyw
pants
pants-reviews
patricklaw, stuhood, zundel

Note that ZincCompile retains a much higher default.

Also note that all current Pants repos override this
in pants.ini anyway, so this is primarily so that new
users can get up and running with some sort of sensible
default.

256M was chosen as a compromise between having to little
memory to get anything done and having several nailguns
chew up all RAM on the system.

CI passes: https://travis-ci.org/pantsbuild/pants/builds/123511811

  1. 
      
  2. Maybe it is small, but it isn't an arbitrary setting.

    We have had issues with memory constrained servers running out of memory in tools (and its a cryptic failure)

    Problem with the shader:

    https://groups.google.com/forum/#!searchin/pants-devel/Shading$20of$20tool$20$27zinc$27$20failed$20on$20travis$20CI/pants-devel/_AubJN5BGEw/xZZbOyJfCwAJ

    An attempt to use less memory by John (maybe unrelated)
    https://rbcommons.com/s/twitter/r/2077/diff/1#index_header

    Problem with giving jar-tool tool too much memory?

    https://groups.google.com/forum/#!searchin/pants-devel/jar-tool/pants-devel/DJsslqRuSjA/mHC00V5vLm8J

    1. Sure, but Square overrides this to 300M, Twitter to 1G and Foursquare to 2G. So why 64M? It clearly isn't enough for real-world use. And why jar-tool, and not other tools? It's not even clear to me from the links above whether the problem was due to too high a setting or too low a setting...

      I'd like us to lock down sensible jvm_options defaults before 1.0. If we're going to tune -Xmx for individual tools then we need to pick reasonable values. Otherwise let's leave it blank. But I'd like this to be consistent, either way.

      I'm still not clear on whether you're saying that you do or do not want the defaults to be set, and if so then to which values. Could you clarify?

    2. One solution for this issue is for me to add documentation that says what to do with the error:

      tool 'xxx' with main class org.pantsbuild.xxx.Main for yyy.zzz failed with exit code -9
      

      The error -9 message reported in at least one of those messages is from the Linux OOM memory killer. This means that a process is trying to malloc more memory than there is virutal memory available on the system. Pants quickly runs out of memory if you try to run it on a small AWS container if you want to try replicating the problem for yourself.

      I'm not saying that we shouldn't bump up the 64M setting, even though it seems to work fine on twitter commons and the pants build itself.

      I think this came out of an effort to try to reign in the resource consumption of pants when running under nailgun. I did this for jar-tool and it was probably too aggressive. But if you think it is harmless to just bump up every tool higher and higher, that's a misconception and will cause problems with new adopters too. When you bump into a "heap too small" problem, the jvm that is using too much heap dies with an exception. When you bump into "tool using too much memory" issue, the OOM memory killer chooses a process to kill. It happens at different points in the build and could be any process your user is running, which makes it very difficult to diagnose.

    3. I'd be fine with a documentation-driven solution, but if we continue to set the memory settings for jar-tool differently than the default then let's definitely document that too (in the code and the docs).

      But it sounds from what you're saying that we should actually explicitly set the default for all JVMs, but that we should set it low-ish (say 256M). Because apparently from Java 7 onwards, if -Xmx is unspecified then the JVM will use up to 1GB, which is probably too high when running multiple nailguns.

    4. Yes, I think setting a default heap setting everywhere is a wise move. If you are using a server JVM, my understanding is that the jvm will default to just taking some proportion of all the resources it finds available on the machine. The bigger the machine the bigger the heap and # of gc threads, etc it allocates! We had production issues with the default # of GC threads trying to co-locate multiple services running in different JVM instances.

    5. Also, I will agree that 256M is a good starting point.

    6. Fun fact - Java considers my 5 year old macbook air to be a server class machine for memory management purposes...

    7. OK, set to 256m as discussed.

  3. 
      
  1. Let's wait for some other folks to chime in before merging.

    It is worth noting that zinc explicitly overrides this setting, so this shouldn't impact scala/java compilation.

  2. 
      
  1. If it is mostly for new users, would it be more obvious to override this in pants.ini, so they can know that's something changable.

    1. Well the whole point is to have some sensible default without requiring users to mess around with pants.ini. Clearly sooner or later most people will need to make changes in pants.ini, but the underlying defaults should be sound.

    2. @yicheng Its been a long term goal to get pants to work well 'out of the box' with an empty pants.ini and no need for BUILD.tools.

    3. sounds good. thanks!

  2. 
      
  1. Ship It!
  2. 
      
  1. Ship It!
  2. 
      
Review request changed

Status: Closed (submitted)

Change Summary:

416423f702edf870d9c622147f9f1aa620a77cce

Loading...