[system-health] Unify SH stories interface

[system-health] Unify SH story set interface

This patch modifies SH stories so that all of their users instantiate
them via page_sets.SystemHealthStorySet, which has the following
parameters:

  platform:                 'desktop' or 'mobile'  (mandatory)
  case:                     'load', 'browse', ...  (default: None)
  take_memory_measurement:  True or False          (default: False)

These parameters will be used by existing benchmarks as follows:

                                                             take_
  Benchmark name                        platform   case      memory_
                                                             measurement
  ----------------------------------------------------------------------
  system_health.memory_desktop          'desktop'  'load'    True
  system_health.memory_mobile           'mobile'   'load'    True
  battor.system_health_loading_desktop  'desktop'  'load'    False
  battor.system_health_loading_mobile   'mobile'   'load'    False
  v8.browsing_desktop                   'desktop'  'browse'  False
  v8.browsing_mobile                    'mobile'   'browse'  False

The case of the memory system health benchmarks will be changed to None
in a subsequent patch, i.e. they will run all SH stories. As a result,
all SH stories will also run in the SH smoke tests in the Chromium CQ.

BUG=589726
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq

Committed: https://crrev.com/fc79a085aec3b72004cbc99864291801e2b1ceca
Cr-Commit-Position: refs/heads/master@{#405753}

[petrcermak, 2016-07-14 15:23:06]

Description was changed from

==========
[system-health] Unify SH stories interface

BUG=589726
==========

to

==========
[system-health] Unify SH stories interface

BUG=589726
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq
==========

[petrcermak, 2016-07-14 15:27:28]

Patchset #1 (id:1) has been deleted

[petrcermak, 2016-07-14 15:35:33]

Patchset #1 (id:20001) has been deleted

[petrcermak, 2016-07-14 15:52:22]

Description was changed from

==========
[system-health] Unify SH stories interface

BUG=589726
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq
==========

to

==========
[system-health] Unify SH story set interface

This patch modifies SH stories so that all of their users instantiate
them via page_sets.SystemHealthStorySet, which has the following
parameters:

  platform:                 'desktop' or 'mobile'  (mandatory)
  story_name_regex:         e.g. r'media:news:.*'  (default: r'.*')
  take_memory_measurement:  True or False          (default: False)

These parameters will be used by existing benchmarks as follows:

                                                 story_       take_
  Benchmark name                       platform  name_        memory_
                                                 regex_       measurem.
  ---------------------------------------------------------------------
  system_health.memory_desktop         'desktop' r'load:.*'   True
  system_health.memory_mobile          'mobile'  r'load:.*'   True
  battor.system_health_loading_desktop 'desktop' r'load:.*'   False
  battor.system_health_loading_mobile  'mobile'  r'load:.*''  False
  v8.browsing_desktop                  'desktop' r'browse:.*' False
  v8.browsing_mobile                   'mobile'  r'browse:.*' False

The story name regular expression of the memory system health benchmarks
will be changed to r'.*' in a subsequent patch, i.e. they will run all
SH stories. As a result, all SH stories will also run in the SH smoke
tests in the Chromium CQ.

BUG=589726
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq
==========

[petrcermak, 2016-07-14 15:53:24]

petrcermak@chromium.org changed reviewers:
+ nednguyen@google.com

[petrcermak, 2016-07-14 15:53:24]

PTAL.

Thanks,
Petr

[nednguyen, 2016-07-14 20:01:49]

https://codereview.chromium.org/2144073004/diff/40001/tools/perf/page_sets/sy...
File tools/perf/page_sets/system_health/system_health_stories.py (right):

https://codereview.chromium.org/2144073004/diff/40001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/system_health_stories.py:20: def
__init__(self, platform, story_name_regex=r'.*',
this story_name_regex is a bit too flexible. I think we want something more
restricted that this. Can you change this to story_type, which acceptable values
are "loading", "browsing"...?

https://codereview.chromium.org/2144073004/diff/40001/tools/perf/page_sets/sy...
File tools/perf/page_sets/system_health/system_health_story.py (right):

https://codereview.chromium.org/2144073004/diff/40001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/system_health_story.py:26: return
cls.__dict__.get('__ABSTRACT_STORY__', False)
nice trick!

[petrcermak, 2016-07-15 09:01:23]

Description was changed from

==========
[system-health] Unify SH story set interface

This patch modifies SH stories so that all of their users instantiate
them via page_sets.SystemHealthStorySet, which has the following
parameters:

  platform:                 'desktop' or 'mobile'  (mandatory)
  story_name_regex:         e.g. r'media:news:.*'  (default: r'.*')
  take_memory_measurement:  True or False          (default: False)

These parameters will be used by existing benchmarks as follows:

                                                 story_       take_
  Benchmark name                       platform  name_        memory_
                                                 regex_       measurem.
  ---------------------------------------------------------------------
  system_health.memory_desktop         'desktop' r'load:.*'   True
  system_health.memory_mobile          'mobile'  r'load:.*'   True
  battor.system_health_loading_desktop 'desktop' r'load:.*'   False
  battor.system_health_loading_mobile  'mobile'  r'load:.*''  False
  v8.browsing_desktop                  'desktop' r'browse:.*' False
  v8.browsing_mobile                   'mobile'  r'browse:.*' False

The story name regular expression of the memory system health benchmarks
will be changed to r'.*' in a subsequent patch, i.e. they will run all
SH stories. As a result, all SH stories will also run in the SH smoke
tests in the Chromium CQ.

BUG=589726
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq
==========

to

==========
[system-health] Unify SH story set interface

This patch modifies SH stories so that all of their users instantiate
them via page_sets.SystemHealthStorySet, which has the following
parameters:

  platform:                 'desktop' or 'mobile'  (mandatory)
  case:                     'load', 'browse', ...  (default: None)
  take_memory_measurement:  True or False          (default: False)

These parameters will be used by existing benchmarks as follows:

                                                             take_
  Benchmark name                        platform   case      memory_
                                                             measurement
  ----------------------------------------------------------------------
  system_health.memory_desktop          'desktop'  'load'    True
  system_health.memory_mobile           'mobile'   'load'    True
  battor.system_health_loading_desktop  'desktop'  'load'    False
  battor.system_health_loading_mobile   'mobile'   'load'    False
  v8.browsing_desktop                   'desktop'  'browse'  False
  v8.browsing_mobile                    'mobile'   'browse'  False

The case of the memory system health benchmarks will be changed to None
in a subsequent patch, i.e. they will run all SH stories. As a result,
all SH stories will also run in the SH smoke tests in the Chromium CQ.

BUG=589726
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq
==========

[petrcermak, 2016-07-15 09:01:43]

Thanks for your comments. PTAL.

Petr

https://codereview.chromium.org/2144073004/diff/40001/tools/perf/page_sets/sy...
File tools/perf/page_sets/system_health/system_health_stories.py (right):

https://codereview.chromium.org/2144073004/diff/40001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/system_health_stories.py:20: def
__init__(self, platform, story_name_regex=r'.*',
On 2016/07/14 20:01:49, nednguyen wrote:
> this story_name_regex is a bit too flexible. I think we want something more
> restricted that this. Can you change this to story_type, which acceptable
values
> are "loading", "browsing"...?

Done. Note that this means that whenever we add a story to a particular case
group (e.g. the new 'browse' story in
https://codereview.chromium.org/2148843002/), it will have to be enabled across
all benchmarks using the relevant story set simultaneously (e.g.
'v8.browsing_*').

https://codereview.chromium.org/2144073004/diff/40001/tools/perf/page_sets/sy...
File tools/perf/page_sets/system_health/system_health_story.py (right):

https://codereview.chromium.org/2144073004/diff/40001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/system_health_story.py:26: return
cls.__dict__.get('__ABSTRACT_STORY__', False)
On 2016/07/14 20:01:49, nednguyen wrote:
> nice trick!

:-)

[nednguyen, 2016-07-15 10:48:29]

https://codereview.chromium.org/2144073004/diff/40001/tools/perf/page_sets/sy...
File tools/perf/page_sets/system_health/system_health_stories.py (right):

https://codereview.chromium.org/2144073004/diff/40001/tools/perf/page_sets/sy...
tools/perf/page_sets/system_health/system_health_stories.py:20: def
__init__(self, platform, story_name_regex=r'.*',
On 2016/07/15 09:01:43, petrcermak wrote:
> On 2016/07/14 20:01:49, nednguyen wrote:
> > this story_name_regex is a bit too flexible. I think we want something more
> > restricted that this. Can you change this to story_type, which acceptable
> values
> > are "loading", "browsing"...?
> 
> Done. Note that this means that whenever we add a story to a particular case
> group (e.g. the new 'browse' story in
> https://codereview.chromium.org/2148843002/), it will have to be enabled
across
> all benchmarks using the relevant story set simultaneously (e.g.
> 'v8.browsing_*').

If it's relevant to browsing, why can't they name this with "browser:..." to get
picked up by browser case?

The biggest down side to this is it removes the possibility to combine groups. 
I initially didn't like the regex on the whole story name strings because people
can easily misuse it, let keep this for now then.

[petrcermak, 2016-07-15 11:11:10]

On 2016/07/15 10:48:29, nednguyen wrote:
>
https://codereview.chromium.org/2144073004/diff/40001/tools/perf/page_sets/sy...
> File tools/perf/page_sets/system_health/system_health_stories.py (right):
> 
>
https://codereview.chromium.org/2144073004/diff/40001/tools/perf/page_sets/sy...
> tools/perf/page_sets/system_health/system_health_stories.py:20: def
> __init__(self, platform, story_name_regex=r'.*',
> On 2016/07/15 09:01:43, petrcermak wrote:
> > On 2016/07/14 20:01:49, nednguyen wrote:
> > > this story_name_regex is a bit too flexible. I think we want something
more
> > > restricted that this. Can you change this to story_type, which acceptable
> > values
> > > are "loading", "browsing"...?
> > 
> > Done. Note that this means that whenever we add a story to a particular case
> > group (e.g. the new 'browse' story in
> > https://codereview.chromium.org/2148843002/), it will have to be enabled
> across
> > all benchmarks using the relevant story set simultaneously (e.g.
> > 'v8.browsing_*').
> 
> If it's relevant to browsing, why can't they name this with "browser:..." to
get
> picked up by browser case?
> 
> The biggest down side to this is it removes the possibility to combine groups.

> I initially didn't like the regex on the whole story name strings because
people
> can easily misuse it, let keep this for now then.

So do you want to keep the current solution (string) or the original one
(regex)? Can I has lg2m?

Thanks,
Petr

[nednguyen, 2016-07-15 11:45:33]

lgtm

For some reason I thought I did, sorry :P

[petrcermak, 2016-07-15 11:55:51]

The CQ bit was checked by petrcermak@chromium.org

[commit-bot: I haz the power, 2016-07-15 11:56:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-15 13:42:56]

Description was changed from

==========
[system-health] Unify SH story set interface

This patch modifies SH stories so that all of their users instantiate
them via page_sets.SystemHealthStorySet, which has the following
parameters:

  platform:                 'desktop' or 'mobile'  (mandatory)
  case:                     'load', 'browse', ...  (default: None)
  take_memory_measurement:  True or False          (default: False)

These parameters will be used by existing benchmarks as follows:

                                                             take_
  Benchmark name                        platform   case      memory_
                                                             measurement
  ----------------------------------------------------------------------
  system_health.memory_desktop          'desktop'  'load'    True
  system_health.memory_mobile           'mobile'   'load'    True
  battor.system_health_loading_desktop  'desktop'  'load'    False
  battor.system_health_loading_mobile   'mobile'   'load'    False
  v8.browsing_desktop                   'desktop'  'browse'  False
  v8.browsing_mobile                    'mobile'   'browse'  False

The case of the memory system health benchmarks will be changed to None
in a subsequent patch, i.e. they will run all SH stories. As a result,
all SH stories will also run in the SH smoke tests in the Chromium CQ.

BUG=589726
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq
==========

to

==========
[system-health] Unify SH story set interface

This patch modifies SH stories so that all of their users instantiate
them via page_sets.SystemHealthStorySet, which has the following
parameters:

  platform:                 'desktop' or 'mobile'  (mandatory)
  case:                     'load', 'browse', ...  (default: None)
  take_memory_measurement:  True or False          (default: False)

These parameters will be used by existing benchmarks as follows:

                                                             take_
  Benchmark name                        platform   case      memory_
                                                             measurement
  ----------------------------------------------------------------------
  system_health.memory_desktop          'desktop'  'load'    True
  system_health.memory_mobile           'mobile'   'load'    True
  battor.system_health_loading_desktop  'desktop'  'load'    False
  battor.system_health_loading_mobile   'mobile'   'load'    False
  v8.browsing_desktop                   'desktop'  'browse'  False
  v8.browsing_mobile                    'mobile'   'browse'  False

The case of the memory system health benchmarks will be changed to None
in a subsequent patch, i.e. they will run all SH stories. As a result,
all SH stories will also run in the SH smoke tests in the Chromium CQ.

BUG=589726
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq
==========

[commit-bot: I haz the power, 2016-07-15 13:42:57]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2016-07-15 13:42:59]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-15 13:44:30]

Description was changed from

==========
[system-health] Unify SH story set interface

This patch modifies SH stories so that all of their users instantiate
them via page_sets.SystemHealthStorySet, which has the following
parameters:

  platform:                 'desktop' or 'mobile'  (mandatory)
  case:                     'load', 'browse', ...  (default: None)
  take_memory_measurement:  True or False          (default: False)

These parameters will be used by existing benchmarks as follows:

                                                             take_
  Benchmark name                        platform   case      memory_
                                                             measurement
  ----------------------------------------------------------------------
  system_health.memory_desktop          'desktop'  'load'    True
  system_health.memory_mobile           'mobile'   'load'    True
  battor.system_health_loading_desktop  'desktop'  'load'    False
  battor.system_health_loading_mobile   'mobile'   'load'    False
  v8.browsing_desktop                   'desktop'  'browse'  False
  v8.browsing_mobile                    'mobile'   'browse'  False

The case of the memory system health benchmarks will be changed to None
in a subsequent patch, i.e. they will run all SH stories. As a result,
all SH stories will also run in the SH smoke tests in the Chromium CQ.

BUG=589726
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq
==========

to

==========
[system-health] Unify SH story set interface

This patch modifies SH stories so that all of their users instantiate
them via page_sets.SystemHealthStorySet, which has the following
parameters:

  platform:                 'desktop' or 'mobile'  (mandatory)
  case:                     'load', 'browse', ...  (default: None)
  take_memory_measurement:  True or False          (default: False)

These parameters will be used by existing benchmarks as follows:

                                                             take_
  Benchmark name                        platform   case      memory_
                                                             measurement
  ----------------------------------------------------------------------
  system_health.memory_desktop          'desktop'  'load'    True
  system_health.memory_mobile           'mobile'   'load'    True
  battor.system_health_loading_desktop  'desktop'  'load'    False
  battor.system_health_loading_mobile   'mobile'   'load'    False
  v8.browsing_desktop                   'desktop'  'browse'  False
  v8.browsing_mobile                    'mobile'   'browse'  False

The case of the memory system health benchmarks will be changed to None
in a subsequent patch, i.e. they will run all SH stories. As a result,
all SH stories will also run in the SH smoke tests in the Chromium CQ.

BUG=589726
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq

Committed: https://crrev.com/fc79a085aec3b72004cbc99864291801e2b1ceca
Cr-Commit-Position: refs/heads/master@{#405753}
==========

[commit-bot: I haz the power, 2016-07-15 13:44:31]

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

[ulan, 2016-07-18 11:10:46]

ulan@chromium.org changed reviewers:
+ ulan@chromium.org

[ulan, 2016-07-18 11:10:47]

https://codereview.chromium.org/2144073004/diff/60001/tools/perf/page_sets/da...
File tools/perf/page_sets/data/memory_system_health_mobile.json (right):

https://codereview.chromium.org/2144073004/diff/60001/tools/perf/page_sets/da...
tools/perf/page_sets/data/memory_system_health_mobile.json:68:
"browsing_mobile_000.wpr": [
Looks like this broke record_wpr:

Traceback (most recent call last):
  <module> at /usr/local/google/home/ulan/chrome/src/tools/perf/record_wpr:20
    sys.exit(record_wpr.Main(environment=chromium_config.ChromiumConfig()))
  Main at
/usr/local/google/home/ulan/chrome/src/third_party/catapult/telemetry/telemetry/record_wpr.py:282
    wpr_recorder.HandleResults(results, args.upload)
  HandleResults at
/usr/local/google/home/ulan/chrome/src/third_party/catapult/telemetry/telemetry/record_wpr.py:236
    upload_to_cloud_storage)
  AddRecordedStories at
/usr/local/google/home/ulan/chrome/src/third_party/catapult/telemetry/telemetry/wpr/archive_info.py:123
    (target_wpr_file, target_wpr_file_path) = self._NextWprFileName()
  _NextWprFileName at
/usr/local/google/home/ulan/chrome/src/third_party/catapult/telemetry/telemetry/wpr/archive_info.py:202
    ', doesn\'t begin with ' + base)
Exception: Illegal wpr file name browsing_desktop_001.wpr, doesn't begin with
memory_system_health_desktop


Would renaming browsing_desktop_001.wpr to memory_system_health_desktop_NNN.wpr
and re-uploading work?
Or maybe change archive_info.py to skip the check?

Let me know if that doesn't work and I will re-record the story set.

[nednguyen, 2016-07-18 11:18:26]

On 2016/07/18 11:10:47, ulan wrote:
>
https://codereview.chromium.org/2144073004/diff/60001/tools/perf/page_sets/da...
> File tools/perf/page_sets/data/memory_system_health_mobile.json (right):
> 
>
https://codereview.chromium.org/2144073004/diff/60001/tools/perf/page_sets/da...
> tools/perf/page_sets/data/memory_system_health_mobile.json:68:
> "browsing_mobile_000.wpr": [
> Looks like this broke record_wpr:
> 
> Traceback (most recent call last):
>   <module> at /usr/local/google/home/ulan/chrome/src/tools/perf/record_wpr:20
>     sys.exit(record_wpr.Main(environment=chromium_config.ChromiumConfig()))
>   Main at
>
/usr/local/google/home/ulan/chrome/src/third_party/catapult/telemetry/telemetry/record_wpr.py:282
>     wpr_recorder.HandleResults(results, args.upload)
>   HandleResults at
>
/usr/local/google/home/ulan/chrome/src/third_party/catapult/telemetry/telemetry/record_wpr.py:236
>     upload_to_cloud_storage)
>   AddRecordedStories at
>
/usr/local/google/home/ulan/chrome/src/third_party/catapult/telemetry/telemetry/wpr/archive_info.py:123
>     (target_wpr_file, target_wpr_file_path) = self._NextWprFileName()
>   _NextWprFileName at
>
/usr/local/google/home/ulan/chrome/src/third_party/catapult/telemetry/telemetry/wpr/archive_info.py:202
>     ', doesn\'t begin with ' + base)
> Exception: Illegal wpr file name browsing_desktop_001.wpr, doesn't begin with
> memory_system_health_desktop
> 
> 
> Would renaming browsing_desktop_001.wpr to
memory_system_health_desktop_NNN.wpr
> and re-uploading work?
> Or maybe change archive_info.py to skip the check?
> 
> Let me know if that doesn't work and I will re-record the story set.

I think renaming should work. Let keep the archive_info behavior for now.

[ulan, 2016-07-18 13:10:25]

On 2016/07/18 11:18:26, nednguyen wrote:
> On 2016/07/18 11:10:47, ulan wrote:
> >
>
https://codereview.chromium.org/2144073004/diff/60001/tools/perf/page_sets/da...
> > File tools/perf/page_sets/data/memory_system_health_mobile.json (right):
> > 
> >
>
https://codereview.chromium.org/2144073004/diff/60001/tools/perf/page_sets/da...
> > tools/perf/page_sets/data/memory_system_health_mobile.json:68:
> > "browsing_mobile_000.wpr": [
> > Looks like this broke record_wpr:
> > 
> > Traceback (most recent call last):
> >   <module> at
/usr/local/google/home/ulan/chrome/src/tools/perf/record_wpr:20
> >     sys.exit(record_wpr.Main(environment=chromium_config.ChromiumConfig()))
> >   Main at
> >
>
/usr/local/google/home/ulan/chrome/src/third_party/catapult/telemetry/telemetry/record_wpr.py:282
> >     wpr_recorder.HandleResults(results, args.upload)
> >   HandleResults at
> >
>
/usr/local/google/home/ulan/chrome/src/third_party/catapult/telemetry/telemetry/record_wpr.py:236
> >     upload_to_cloud_storage)
> >   AddRecordedStories at
> >
>
/usr/local/google/home/ulan/chrome/src/third_party/catapult/telemetry/telemetry/wpr/archive_info.py:123
> >     (target_wpr_file, target_wpr_file_path) = self._NextWprFileName()
> >   _NextWprFileName at
> >
>
/usr/local/google/home/ulan/chrome/src/third_party/catapult/telemetry/telemetry/wpr/archive_info.py:202
> >     ', doesn\'t begin with ' + base)
> > Exception: Illegal wpr file name browsing_desktop_001.wpr, doesn't begin
with
> > memory_system_health_desktop
> > 
> > 
> > Would renaming browsing_desktop_001.wpr to
> memory_system_health_desktop_NNN.wpr
> > and re-uploading work?
> > Or maybe change archive_info.py to skip the check?
> > 
> > Let me know if that doesn't work and I will re-record the story set.
> 
> I think renaming should work. Let keep the archive_info behavior for now.

On the second thought, if we rename the archive files and upload them using the
same SHA, then this will probably break bots?
Since they will have the old json files until the CL with new json files lands.

[nednguyen, 2016-07-18 13:14:23]

On 2016/07/18 13:10:25, ulan wrote:
> On 2016/07/18 11:18:26, nednguyen wrote:
> > On 2016/07/18 11:10:47, ulan wrote:
> > >
> >
>
https://codereview.chromium.org/2144073004/diff/60001/tools/perf/page_sets/da...
> > > File tools/perf/page_sets/data/memory_system_health_mobile.json (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2144073004/diff/60001/tools/perf/page_sets/da...
> > > tools/perf/page_sets/data/memory_system_health_mobile.json:68:
> > > "browsing_mobile_000.wpr": [
> > > Looks like this broke record_wpr:
> > > 
> > > Traceback (most recent call last):
> > >   <module> at
> /usr/local/google/home/ulan/chrome/src/tools/perf/record_wpr:20
> > >    
sys.exit(record_wpr.Main(environment=chromium_config.ChromiumConfig()))
> > >   Main at
> > >
> >
>
/usr/local/google/home/ulan/chrome/src/third_party/catapult/telemetry/telemetry/record_wpr.py:282
> > >     wpr_recorder.HandleResults(results, args.upload)
> > >   HandleResults at
> > >
> >
>
/usr/local/google/home/ulan/chrome/src/third_party/catapult/telemetry/telemetry/record_wpr.py:236
> > >     upload_to_cloud_storage)
> > >   AddRecordedStories at
> > >
> >
>
/usr/local/google/home/ulan/chrome/src/third_party/catapult/telemetry/telemetry/wpr/archive_info.py:123
> > >     (target_wpr_file, target_wpr_file_path) = self._NextWprFileName()
> > >   _NextWprFileName at
> > >
> >
>
/usr/local/google/home/ulan/chrome/src/third_party/catapult/telemetry/telemetry/wpr/archive_info.py:202
> > >     ', doesn\'t begin with ' + base)
> > > Exception: Illegal wpr file name browsing_desktop_001.wpr, doesn't begin
> with
> > > memory_system_health_desktop
> > > 
> > > 
> > > Would renaming browsing_desktop_001.wpr to
> > memory_system_health_desktop_NNN.wpr
> > > and re-uploading work?
> > > Or maybe change archive_info.py to skip the check?
> > > 
> > > Let me know if that doesn't work and I will re-record the story set.
> > 
> > I think renaming should work. Let keep the archive_info behavior for now.
> 
> On the second thought, if we rename the archive files and upload them using
the
> same SHA, then this will probably break bots?
> Since they will have the old json files until the CL with new json files
lands.

As long as we just create new copies and not delete the old ones, I think that
shouldn't cause a problem?

[ulan, 2016-07-18 13:47:45]

On 2016/07/18 13:14:23, nednguyen wrote:
> On 2016/07/18 13:10:25, ulan wrote:
> > On 2016/07/18 11:18:26, nednguyen wrote:
> > > On 2016/07/18 11:10:47, ulan wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/2144073004/diff/60001/tools/perf/page_sets/da...
> > > > File tools/perf/page_sets/data/memory_system_health_mobile.json (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2144073004/diff/60001/tools/perf/page_sets/da...
> > > > tools/perf/page_sets/data/memory_system_health_mobile.json:68:
> > > > "browsing_mobile_000.wpr": [
> > > > Looks like this broke record_wpr:
> > > > 
> > > > Traceback (most recent call last):
> > > >   <module> at
> > /usr/local/google/home/ulan/chrome/src/tools/perf/record_wpr:20
> > > >    
> sys.exit(record_wpr.Main(environment=chromium_config.ChromiumConfig()))
> > > >   Main at
> > > >
> > >
> >
>
/usr/local/google/home/ulan/chrome/src/third_party/catapult/telemetry/telemetry/record_wpr.py:282
> > > >     wpr_recorder.HandleResults(results, args.upload)
> > > >   HandleResults at
> > > >
> > >
> >
>
/usr/local/google/home/ulan/chrome/src/third_party/catapult/telemetry/telemetry/record_wpr.py:236
> > > >     upload_to_cloud_storage)
> > > >   AddRecordedStories at
> > > >
> > >
> >
>
/usr/local/google/home/ulan/chrome/src/third_party/catapult/telemetry/telemetry/wpr/archive_info.py:123
> > > >     (target_wpr_file, target_wpr_file_path) = self._NextWprFileName()
> > > >   _NextWprFileName at
> > > >
> > >
> >
>
/usr/local/google/home/ulan/chrome/src/third_party/catapult/telemetry/telemetry/wpr/archive_info.py:202
> > > >     ', doesn\'t begin with ' + base)
> > > > Exception: Illegal wpr file name browsing_desktop_001.wpr, doesn't begin
> > with
> > > > memory_system_health_desktop
> > > > 
> > > > 
> > > > Would renaming browsing_desktop_001.wpr to
> > > memory_system_health_desktop_NNN.wpr
> > > > and re-uploading work?
> > > > Or maybe change archive_info.py to skip the check?
> > > > 
> > > > Let me know if that doesn't work and I will re-record the story set.
> > > 
> > > I think renaming should work. Let keep the archive_info behavior for now.
> > 
> > On the second thought, if we rename the archive files and upload them using
> the
> > same SHA, then this will probably break bots?
> > Since they will have the old json files until the CL with new json files
> lands.
> 
> As long as we just create new copies and not delete the old ones, I think that
> shouldn't cause a problem?

I uploaded https://codereview.chromium.org/2161583003/