Make javac invocations incremental when possible

Adds an --incremental flag to javac.py and a GN flag to enable it

Overhauls md5_check.py to have it track exactly what files change,
as well as what subfiles within zip files change.

Uses this information in javac.py to compile only source files that have
changed since the last invokation.

Timings for the javac.py step of chrome_java with one file changed:
Before:
real    0m11.381s
user    0m27.881s
sys     0m1.217s

After:
real    0m3.511s
user    0m4.624s
sys     0m0.631s

BUG=533442
Committed: https://crrev.com/d8cef607861d43744b652410aff6f5629181a0f1
Cr-Commit-Position: refs/heads/master@{#350680}

[agrieve, 2015-09-22 20:16:56]

agrieve@chromium.org changed reviewers:
+ jbudorick@chromium.org

[jbudorick, 2015-09-23 00:26:20]

lgtm w/ nits

You weren't kidding, this is a big one.

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/jar.py
File build/android/gyp/jar.py (right):

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/jar.p...
build/android/gyp/jar.py:36: def JarDirectory(classes_dir, jar_path,
manifest_file=None, predicate=None):
Nice.

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/javac.py
File build/android/gyp/javac.py (right):

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/javac...
build/android/gyp/javac.py:116: path_without_suffix =
re.sub(r'(?:\$|\.class)[^/]*$', '', path)
path[:-len('.class')]

or something along those lines? We already know that path ends with '.class' by
this point.

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/javac...
build/android/gyp/javac.py:118: return ret
nit: just

  return not any(path_without_suffix in p for p in java_files)

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/util/...
File build/android/gyp/util/build_utils.py (right):

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/util/...
build/android/gyp/util/build_utils.py:213: if predicate is not None and not
predicate(name):
I don't think functions can be interpreted as false, so I think this would be
fine as

  if predicate and not predicate(name):

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/util/...
File build/android/gyp/util/md5_check.py (right):

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/util/...
build/android/gyp/util/md5_check.py:108: self.old_metadata is None or
not self.old_metadata?

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/util/...
build/android/gyp/util/md5_check.py:213: return 'I have no idea what changed
(there is a bug).'
hahaha

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/util/...
build/android/gyp/util/md5_check.py:231: #       "tag": "{MD5}",
nit: missing closing }

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/util/...
build/android/gyp/util/md5_check.py:244: def FromFile(cls, fileobj):
not Read/Write or FromFile/ToFile? :(

[agrieve, 2015-09-23 02:07:56]

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/jar.py
File build/android/gyp/jar.py (right):

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/jar.p...
build/android/gyp/jar.py:36: def JarDirectory(classes_dir, jar_path,
manifest_file=None, predicate=None):
On 2015/09/23 00:26:20, jbudorick wrote:
> Nice.

Acknowledged.

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/javac.py
File build/android/gyp/javac.py (right):

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/javac...
build/android/gyp/javac.py:116: path_without_suffix =
re.sub(r'(?:\$|\.class)[^/]*$', '', path)
On 2015/09/23 00:26:20, jbudorick wrote:
> path[:-len('.class')]
> 
> or something along those lines? We already know that path ends with '.class'
by
> this point.

Hmm, yeah, sorely lacking a comment here. 
Added:
  # Two challenges exist here:
  # 1. |java_files| have prefixes that are not represented in the the jar paths.
  # 2. A single .java file results in multiple .class files when it contains
  #    nested classes.
  # Here's an example:
  #   source path: ../../base/android/java/src/org/chromium/Foo.java
  #   jar paths: org/chromium/Foo.class, org/chromium/Foo$Inner.class
  # To extract only .class files not related to the given .java files, we strip
  # off ".class" and "$*.class" and use a substring match against java_files.

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/javac...
build/android/gyp/javac.py:118: return ret
On 2015/09/23 00:26:20, jbudorick wrote:
> nit: just
> 
>   return not any(path_without_suffix in p for p in java_files)

Done.

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/util/...
File build/android/gyp/util/build_utils.py (right):

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/util/...
build/android/gyp/util/build_utils.py:213: if predicate is not None and not
predicate(name):
On 2015/09/23 00:26:20, jbudorick wrote:
> I don't think functions can be interpreted as false, so I think this would be
> fine as
> 
>   if predicate and not predicate(name):

Done.

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/util/...
File build/android/gyp/util/md5_check.py (right):

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/util/...
build/android/gyp/util/md5_check.py:108: self.old_metadata is None or
On 2015/09/23 00:26:20, jbudorick wrote:
> not self.old_metadata?

Done.

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/util/...
build/android/gyp/util/md5_check.py:231: #       "tag": "{MD5}",
On 2015/09/23 00:26:20, jbudorick wrote:
> nit: missing closing }

Done.

https://codereview.chromium.org/1361733002/diff/20001/build/android/gyp/util/...
build/android/gyp/util/md5_check.py:244: def FromFile(cls, fileobj):
On 2015/09/23 00:26:20, jbudorick wrote:
> not Read/Write or FromFile/ToFile? :(

Done.

[agrieve, 2015-09-24 18:09:43]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2015-09-24 18:09:43]

The patchset sent to the CQ was uploaded after l-g-t-m from
jbudorick@chromium.org
Link to the patchset: https://codereview.chromium.org/1361733002/#ps100001
(title: "add flag and disable by default")

[commit-bot: I haz the power, 2015-09-24 18:10:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1361733002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1361733002/100001

[agrieve, 2015-09-24 19:46:46]

The CQ bit was unchecked by agrieve@chromium.org

[agrieve, 2015-09-24 21:56:39]

The CQ bit was checked by agrieve@chromium.org

[commit-bot: I haz the power, 2015-09-24 21:57:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1361733002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1361733002/100001

[commit-bot: I haz the power, 2015-09-24 22:08:48]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-09-24 22:09:28]

Patchset 6 (id:??) landed as
https://crrev.com/d8cef607861d43744b652410aff6f5629181a0f1
Cr-Commit-Position: refs/heads/master@{#350680}

[cjhopman2, 2015-09-25 06:59:40]

On 2015/09/24 22:09:28, commit-bot: I haz the power wrote:
> Patchset 6 (id:??) landed as
> https://crrev.com/d8cef607861d43744b652410aff6f5629181a0f1
> Cr-Commit-Position: refs/heads/master@{#350680}

I only took a quick look at this, but it may be wrong:

It is possible that changing A.java will require that B.java be rebuilt even if
B.java doesn't change. E.g. A.java contains a primitive constant that is inlined
into B.java.

[agrieve, 2015-09-25 14:22:09]

On 2015/09/25 06:59:40, cjhopman2 wrote:
> On 2015/09/24 22:09:28, commit-bot: I haz the power wrote:
> > Patchset 6 (id:??) landed as
> > https://crrev.com/d8cef607861d43744b652410aff6f5629181a0f1
> > Cr-Commit-Position: refs/heads/master@{#350680}
> 
> I only took a quick look at this, but it may be wrong:
> 
> It is possible that changing A.java will require that B.java be rebuilt even
if
> B.java doesn't change. E.g. A.java contains a primitive constant that is
inlined
> into B.java.

I don't know a thing about constant inlining (so quite likely your are right!).
But even more glaring is that if A.java deletes a symbol used by B.java, B.java
isn't even being recompiled right now. 

Will have to add some javap logic I think. Or if you have other ideas please
share :)

Thanks for pointing this out!

[agrieve, 2015-09-25 15:22:36]

On 2015/09/25 14:22:09, agrieve wrote:
> On 2015/09/25 06:59:40, cjhopman2 wrote:
> > On 2015/09/24 22:09:28, commit-bot: I haz the power wrote:
> > > Patchset 6 (id:??) landed as
> > > https://crrev.com/d8cef607861d43744b652410aff6f5629181a0f1
> > > Cr-Commit-Position: refs/heads/master@{#350680}
> > 
> > I only took a quick look at this, but it may be wrong:
> > 
> > It is possible that changing A.java will require that B.java be rebuilt even
> if
> > B.java doesn't change. E.g. A.java contains a primitive constant that is
> inlined
> > into B.java.
> 
> I don't know a thing about constant inlining (so quite likely your are
right!).
> But even more glaring is that if A.java deletes a symbol used by B.java,
B.java
> isn't even being recompiled right now. 
> 
> Will have to add some javap logic I think. Or if you have other ideas please
> share :)
> 
> Thanks for pointing this out!

Hmm, javap won't help when it comes to detecting constant inlining. Going to
experiment with just grepping other files for references to the changed file to
see if that's viable. It probably still wouldn't catch the case of:

A.foo = 1
inlined to:
B.foo = 1
further inlined to:
C.foo = 1

[agrieve, 2015-09-25 17:34:46]

On 2015/09/25 15:22:36, agrieve wrote:
> On 2015/09/25 14:22:09, agrieve wrote:
> > On 2015/09/25 06:59:40, cjhopman2 wrote:
> > > On 2015/09/24 22:09:28, commit-bot: I haz the power wrote:
> > > > Patchset 6 (id:??) landed as
> > > > https://crrev.com/d8cef607861d43744b652410aff6f5629181a0f1
> > > > Cr-Commit-Position: refs/heads/master@{#350680}
> > > 
> > > I only took a quick look at this, but it may be wrong:
> > > 
> > > It is possible that changing A.java will require that B.java be rebuilt
even
> > if
> > > B.java doesn't change. E.g. A.java contains a primitive constant that is
> > inlined
> > > into B.java.
> > 
> > I don't know a thing about constant inlining (so quite likely your are
> right!).
> > But even more glaring is that if A.java deletes a symbol used by B.java,
> B.java
> > isn't even being recompiled right now. 
> > 
> > Will have to add some javap logic I think. Or if you have other ideas please
> > share :)
> > 
> > Thanks for pointing this out!
> 
> Hmm, javap won't help when it comes to detecting constant inlining. Going to
> experiment with just grepping other files for references to the changed file
to
> see if that's viable. It probably still wouldn't catch the case of:
> 
> A.foo = 1
> inlined to:
> B.foo = 1
> further inlined to:
> C.foo = 1

https://codereview.chromium.org/1373443004/

[Yaron, 2015-09-25 17:59:42]

On 2015/09/25 06:59:40, cjhopman2 wrote:
> On 2015/09/24 22:09:28, commit-bot: I haz the power wrote:
> > Patchset 6 (id:??) landed as
> > https://crrev.com/d8cef607861d43744b652410aff6f5629181a0f1
> > Cr-Commit-Position: refs/heads/master@{#350680}
> 
> I only took a quick look at this, but it may be wrong:
> 
> It is possible that changing A.java will require that B.java be rebuilt even
if
> B.java doesn't change. E.g. A.java contains a primitive constant that is
inlined
> into B.java.

Good catch - thanks for your feedback, Chris! Hope all is well