Convert loose directories in bundle classpath into jars

Review Request #3297 — Created Jan. 5, 2016 and submitted

peiyu
pants
2771
3851, 3304
pants-reviews
nhoward_tw, patricklaw, stuhood, zundel

For large targets, loading resources from jars is more efficient than loading
them as individual files. In our test, for a classpath with 1k+ resource
directories, saving is about 10-20% app startup time.

Note in theory, run and junit can also benefit from this optimization, to make
that happen, we need to prepare junit runner so extracts jars if necessary before
running tests that expect directories, not a priority for now.

https://travis-ci.org/peiyuwang/pants/builds/100423380

  • 0
  • 0
  • 3
  • 0
  • 3
Description From Last Updated
PA
  1. 
      
  2. Accessing "private" attributes is discouraged. If this really shouldn't be provide, then we should expose a property to access .classpaths.

    1. sorry, will fix, copypasta from existing internal hack (on purpose)

    2. Fixed now

  3. The key, val in dict(...).items() seems redundant. What's the point of packing and then unpacking the tuples into a dictionary?

  4. 
      
PE
PE
ZU
  1. Ship It!
  2. 
      
ST
  1. 
      
  2. self.context.targets() is already the closure of the target_roots, so this line is redundant.

    1. removed, was on the wrong assumption that one is with synthetic targets, one without. self.context.targets is already the full closure of target_roots.

  3. src/python/pants/backend/jvm/tasks/bundle_create.py (Diff revision 3)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    Unfortunately, it doesn't look like this task is using cache_target_dirs, or caching at all for that matter (see http://pantsbuild.github.io/dev_tasks.html#enabling-artifact-caching-for-tasks) otherwise it would be trivial to write these in target-specific directories.

    1. Makes sense, I am adding a TODO, seems can be addressed as a follow up task.

    2. Following up here: https://rbcommons.com/s/twitter/r/3304/

  4. Comment is stale, since these aren't temporary anymore... perhaps just 'unique'.

  5. 
      
PE
ST
  1. Thanks Peiyu.

  2. 
      
PE
Review request changed

Status: Closed (submitted)

Change Summary:

Commited as c7097307724b6205e83f1bd96e563773794a75b4

Loading...