Remove the JVM global compile strategy, switch default java compiler to zinc

Review Request #2852 — Created Sept. 21, 2015 and submitted

stuhood
pants
085d21b...
pants-reviews
benjyw, jsirois, nhoward_tw, patricklaw, zundel

On the assumption that Foursquare's switch to the isolated compile strategy will stick, this patch deletes the global strategy.

  • Inline all of the isolated strategy into jvm_compile
  • Merge pants.ini files
  • Delete the global strategy and strategy interface
  • Make zinc the default for java compiles since jmake does not support partitioned analysis
  • Update migrate_config to indicate the global-related options will be ignored

https://github.com/pantsbuild/pants/pull/2227

BE
  1. Thanks for this. Love it!

    Let's just wait a couple more days before merging to master?

    So long global strategy. You won't be missed.

  2. I don't love how gigantic this file is now, but we can clean that up later. Or maybe the new engine version of this will end up being a lot simpler to begin with.

    1. Yea, I didn't do anything at all to collapse it.

      A few things we can do:
      1) AptCompile can be merged in
      2) we don't need to extend GroupTask
      3) we can use an invalidated block per target (or not)
      4) we can switch to cache_target_dirs and remove all manual caching (and deprecate it in pants)
      5) can kill the classes_by_target product

  3. 
      
NH
  1. LGTM. I've got one minor nitpick inline and a question.

    Is changing the default Java compiler necessary for removing global? I'd prefer if that were a separate RB so that it shows up as a line item in the changelog. If it's a necessary part of this change, could you add that to the title of this patch?

    1. It's necessary to get good incremental compile behaviour... jmake is completely unaware of upstream analysis. While Square have apparently been getting it to mostly work, I expect that their incremental compiles have always been full-recompiles instead. We saw a more than 4x speedup internally using zinc for incremental compiles rather than jmake.

      Will edit the title.

    2. I'll note again that this is likely only because of mixed scala-java.  Since they just have java, the difference in speed is probably in the noise.
    3. Perhaps. But if that's the case, then jmake never needed to support incremental compile in the first place, and I find that hard to believe.

    4. Doesn't jmake exist exclusively for incremental compile? Without that it's just a superfluous wrapper around javac, no?

    5. As I recall from way back when I introduced it, jmake's single global analysis worked well for a single global compile with small changes in a core lib.  So many caveats, but if you're pure java and you use global and you work on the same set of targets for a few hours, it pays off.  I think that restricted set of conditions is actually common enough for a dev hacking on a java codebase.  Combined with the relative speed of javac, even changing the target set led to not enough slowdown to be all that noticeable.
  2. rm ref to strategies.

  3. 
      
ST
NH
  1. Thanks for the docstring updates and the title change!

  2. 
      
ZU
  1. 
      
  2. You changed the 'jmake' option to default to false. Maybe it would be good to add a note if someone is using this value because jmake will soon be removed?

  3. Awesome - this will probably speed up CI!

  4. 
      
ST
ST
BE
  1. Ship It!
  2. 
      
ST
ST
Review request changed

Status: Closed (submitted)

Change Summary:

Merged as c4d964d28871954f2927adc564f2c054027c234f

Loading...