Use BufferedOutputStream

Review Request #2270 — Created May 27, 2015 and submitted

sserebryakov
pants
sserebryakov/jar/buffered_os
1596
5ea5282...
pants-reviews
kwlzn, tejal

Using BufferedOutputStream significantly speeds up the JAR creation. See testing section for an example.

Green CI: https://travis-ci.org/pantsbuild/pants/builds/64236409

I used this command for benchmarking (+ flushing caches):

sudo /usr/sbin/purge && ls -alR > /dev/null && ./pants binary --no-use-nailgun macaw-swift/web-app::

Without BOS, the jar-tool step took 105, 112, and 123 seconds.
With BOS, the jar-tool step took 77, 80, and 85 seconds.

JS
  1. 
      
  2. Please add a comment documenting the perf win.  Without something - unit test, perf test, comment - the code is subject to forfeit a year from now when no-one remembers.  It seems to be true that it's the very rare person to track this sort of info down via blame/log.
    1. Added hopefully informative comment.

      Sidenote: what you're saying seems to be true for pretty much any code. By this logic, why shouldn't every line of code be supplied with a comment explaining its existence?

    2. Just to back John up a bit, I've had one or two 'near misses' of important features/fixes almost being reverted out of the codebase. In this case I would say that it isn't obvious by looking at that change that it gives a significant performance gain (in fact, it may not in other environments, we don't have any tasks that take 100+ seconds to create a jar), and that the 'magic number' of 1024 * 1024 has actually been vetted somehow.

    3. Exactly to Eric's reasoning.  Sergey - in your model this code is in fact special.  It can be removed and there is no obvious effect - tests still pass, code still runs, and there is no obvious perf impact unless you're staring at -x output in pantsbuild (we have no large fatjars and other steps dominate on a clean build).  Thanks for leaving a note.
  3. 
      
ST
  1. Ship It!
  2. 
      
SS
JS
  1. Thanks Sergey - this is a nice win.  In master @ https://github.com/pantsbuild/pants/commit/3b3f85a458bb2964a4c6ab9ffef79ce63b77e496
    I'll do a publish presently then send up an upgrade RB when the jar is live on maven central.
    
    Please mark this review as submitted.
    1. The new jar-tool is live: https://repo1.maven.org/maven2/org/pantsbuild/jar-tool/0.0.5/jar-tool-0.0.5-CHANGELOG.txt
      Upgrade RB here: https://rbcommons.com/s/twitter/r/2278/
  2. 
      
SS
Review request changed

Status: Closed (submitted)

Loading...