Make repeated running of instrumentation tests faster.

Make repeated running of instrumentation tests faster.

Running proguard is slow as mollases. Don't do it unless needed.
This could actually improve bot time a bit as we break out the different
sizes to run tests with.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=169723

[Yaron, 2012-11-21 21:58:12]

Shaving a yak cause I got tired of flaky test iteration being so slow.

[frankf, 2012-11-22 03:03:48]

Good idea!

https://codereview.chromium.org/11421040/diff/2001/build/android/pylib/apk_in...
File build/android/pylib/apk_info.py (right):

https://codereview.chromium.org/11421040/diff/2001/build/android/pylib/apk_in...
build/android/pylib/apk_info.py:57: pickle_name = self._jar_path +
"-proguard.pickle"
single quotes here and below.

https://codereview.chromium.org/11421040/diff/2001/build/android/pylib/apk_in...
build/android/pylib/apk_info.py:59: os.stat(self._jar_path).st_mtime):
os.path.getmtime() is slightly preferred.

https://codereview.chromium.org/11421040/diff/2001/build/android/pylib/apk_in...
build/android/pylib/apk_info.py:67: logging.warning("PICKLE_FORMAT_VERSION has
changed, ignoring cache")
Can you put this in a finally block with above lines in a try. We probably don't
want to propagate any exceptions as this in not critical.

[bulach, 2012-11-22 13:51:20]

nice stuff, thanks yaron! just some suggestions on top of frank's:

https://codereview.chromium.org/11421040/diff/2001/build/android/pylib/apk_in...
File build/android/pylib/apk_info.py (right):

https://codereview.chromium.org/11421040/diff/2001/build/android/pylib/apk_in...
build/android/pylib/apk_info.py:60: logging.info("Loading cached proguard output
from %s", pickle_name)
see below, probably best as:
try:
  with open(pickle_name, 'r') as r:
    d = pickle.loads(r.read())
  if d['VERSION'] == PICKLE_FORMAT_VERSION:
    self._annotation_map = d['ANNOTATION_MAP']
    self._test_methods = d['TEST_METHODS']
    return
except:
  logging.warning('PICKLE_FORMAT_VERSION has changed, ignoring cache')

https://codereview.chromium.org/11421040/diff/2001/build/android/pylib/apk_in...
build/android/pylib/apk_info.py:62: if PICKLE_FORMAT_VERSION ==
unpickler.load():
prefer to pickle one dict, see below..

https://codereview.chromium.org/11421040/diff/2001/build/android/pylib/apk_in...
build/android/pylib/apk_info.py:118: pickler = pickle.Pickler(open(pickle_name,
'w'))
pickle.dumps with a dict works better, no need for Pickler / keep the order
consistent between reader and writer..

so perhaps:
  d = {'VERSION': PICKLE_FORMAT_VERSION,
       'ANNOTATION_MAP': self._annotation_map,
       'TEST_METHODS': self._test_methods}
  with open(pickle_name, 'w') as f:
    f.write(pickle.dumps(d))

[Yaron, 2012-11-26 18:26:14]

https://codereview.chromium.org/11421040/diff/2001/build/android/pylib/apk_in...
File build/android/pylib/apk_info.py (right):

https://codereview.chromium.org/11421040/diff/2001/build/android/pylib/apk_in...
build/android/pylib/apk_info.py:57: pickle_name = self._jar_path +
"-proguard.pickle"
On 2012/11/22 03:03:48, frankf wrote:
> single quotes here and below.

Done.

https://codereview.chromium.org/11421040/diff/2001/build/android/pylib/apk_in...
build/android/pylib/apk_info.py:59: os.stat(self._jar_path).st_mtime):
On 2012/11/22 03:03:48, frankf wrote:
> os.path.getmtime() is slightly preferred.

Done.

https://codereview.chromium.org/11421040/diff/2001/build/android/pylib/apk_in...
build/android/pylib/apk_info.py:60: logging.info("Loading cached proguard output
from %s", pickle_name)
On 2012/11/22 13:51:20, bulach wrote:
> see below, probably best as:
> try:
>   with open(pickle_name, 'r') as r:
>     d = pickle.loads(r.read())
>   if d['VERSION'] == PICKLE_FORMAT_VERSION:
>     self._annotation_map = d['ANNOTATION_MAP']
>     self._test_methods = d['TEST_METHODS']
>     return
> except:
>   logging.warning('PICKLE_FORMAT_VERSION has changed, ignoring cache')

Done.

https://codereview.chromium.org/11421040/diff/2001/build/android/pylib/apk_in...
build/android/pylib/apk_info.py:62: if PICKLE_FORMAT_VERSION ==
unpickler.load():
On 2012/11/22 13:51:20, bulach wrote:
> prefer to pickle one dict, see below..

Done.

https://codereview.chromium.org/11421040/diff/2001/build/android/pylib/apk_in...
build/android/pylib/apk_info.py:67: logging.warning("PICKLE_FORMAT_VERSION has
changed, ignoring cache")
On 2012/11/22 03:03:48, frankf wrote:
> Can you put this in a finally block with above lines in a try. We probably
don't
> want to propagate any exceptions as this in not critical. 

Done.

https://codereview.chromium.org/11421040/diff/2001/build/android/pylib/apk_in...
build/android/pylib/apk_info.py:118: pickler = pickle.Pickler(open(pickle_name,
'w'))
On 2012/11/22 13:51:20, bulach wrote:
> pickle.dumps with a dict works better, no need for Pickler / keep the order
> consistent between reader and writer..
> 
> so perhaps:
>   d = {'VERSION': PICKLE_FORMAT_VERSION,
>        'ANNOTATION_MAP': self._annotation_map,
>        'TEST_METHODS': self._test_methods}
>   with open(pickle_name, 'w') as f:
>     f.write(pickle.dumps(d))

Done.

[frankf, 2012-11-26 19:09:08]

LGTM with an optional change.

https://codereview.chromium.org/11421040/diff/8001/build/android/pylib/apk_in...
File build/android/pylib/apk_info.py (right):

https://codereview.chromium.org/11421040/diff/8001/build/android/pylib/apk_in...
build/android/pylib/apk_info.py:69: logging.warning('PICKLE_FORMAT_VERSION has
changed, ignoring cache')
Optional: For sake of modularity, can you factor out this logic into functions.
Something like:

def _Initialize(self):
  if not _GetCachedProguardData():
    _GetProguardData()

[Yaron, 2012-11-27 18:06:17]

Thanks for review. CQing

https://codereview.chromium.org/11421040/diff/8001/build/android/pylib/apk_in...
File build/android/pylib/apk_info.py (right):

https://codereview.chromium.org/11421040/diff/8001/build/android/pylib/apk_in...
build/android/pylib/apk_info.py:69: logging.warning('PICKLE_FORMAT_VERSION has
changed, ignoring cache')
On 2012/11/26 19:09:08, frankf wrote:
> Optional: For sake of modularity, can you factor out this logic into
functions.
> Something like:
> 
> def _Initialize(self):
>   if not _GetCachedProguardData():
>     _GetProguardData()

Done.

[commit-bot: I haz the power, 2012-11-27 18:07:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yfriedman@chromium.org/11421040/7003

[commit-bot: I haz the power, 2012-11-27 20:30:15]

Change committed as 169723