Make timeline_data indented JSON.

Make timeline_data indented JSON.

BUG=None
TEST=None

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289028

[Dai Mikurube (NOT FULLTIME), 2014-06-06 07:16:05]

Hi nednguyen,

Could you ptal?

[nednguyen(REVIEW IN OTHER ACC), 2014-07-21 19:46:53]

On 2014/06/06 07:16:05, Dai Mikurube wrote:
> Hi nednguyen,
> 
> Could you ptal?

Oopss. I only know about this patch when I open my chromium account. :-(

Will review this as soon as possible.

[nednguyen, 2014-07-21 19:51:34]

lgtm

Thanks for this patch. I always want to do this for such a long time.

https://codereview.chromium.org/315393004/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/core/backends/chrome/tracing_timeline_data.py
(right):

https://codereview.chromium.org/315393004/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/backends/chrome/tracing_timeline_data.py:19: def
Serialize(self, f, indent=None):
Can we set the default value of "indent" to 4?

[Dai Mikurube (NOT FULLTIME), 2014-07-22 06:39:46]

Thanks, Ned. Ah, sorry, I'll use your @google.com from the next time.

Committing!

https://codereview.chromium.org/315393004/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/core/backends/chrome/tracing_timeline_data.py
(right):

https://codereview.chromium.org/315393004/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/backends/chrome/tracing_timeline_data.py:19: def
Serialize(self, f, indent=None):
On 2014/07/21 19:51:34, nednguyen wrote:
> Can we set the default value of "indent" to 4?

Done.

[Dai Mikurube (NOT FULLTIME), 2014-07-22 06:39:59]

The CQ bit was checked by dmikurube@chromium.org

[commit-bot: I haz the power, 2014-07-22 06:40:23]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dmikurube@chromium.org/315393004/80001

[commit-bot: I haz the power, 2014-07-22 08:31:26]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-07-22 09:49:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-22 09:49:05]

Try jobs failed on following builders:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[Dai Mikurube (NOT FULLTIME), 2014-07-22 09:50:51]

The CQ bit was checked by dmikurube@chromium.org

[commit-bot: I haz the power, 2014-07-22 09:52:01]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dmikurube@chromium.org/315393004/80001

[commit-bot: I haz the power, 2014-07-22 10:36:29]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-07-22 11:54:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-22 11:54:11]

Try jobs failed on following builders:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[Dai Mikurube (NOT FULLTIME), 2014-07-22 15:21:09]

The CQ bit was checked by dmikurube@chromium.org

[commit-bot: I haz the power, 2014-07-22 15:22:10]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dmikurube@chromium.org/315393004/80001

[nednguyen, 2014-07-22 15:25:19]

The CQ bit was unchecked by nednguyen@google.com

[nednguyen, 2014-07-22 15:34:25]

Sorry, a few more comments after rebasing.

https://codereview.chromium.org/315393004/diff/80001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/timeline/timeline_data.py (right):

https://codereview.chromium.org/315393004/diff/80001/tools/telemetry/telemetr...
tools/telemetry/telemetry/timeline/timeline_data.py:9: def Serialize(self, f,
indent=4):
I don't like this indent param in the Serialize() method of TimelineData
abstract interface. This would make no sense if the underlined data is not
json-compatible.

Can we simplify the whole situation by always indenting the json file with
indent=4?

https://codereview.chromium.org/315393004/diff/80001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/web_perf/timeline_based_measurement.py (right):

https://codereview.chromium.org/315393004/diff/80001/tools/telemetry/telemetr...
tools/telemetry/telemetry/web_perf/timeline_based_measurement.py:154:
help=('Indent to save the trace after the run.'))
Can you default this to 4 too?

[Dai Mikurube (NOT FULLTIME), 2014-07-23 04:59:06]

https://codereview.chromium.org/315393004/diff/80001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/timeline/timeline_data.py (right):

https://codereview.chromium.org/315393004/diff/80001/tools/telemetry/telemetr...
tools/telemetry/telemetry/timeline/timeline_data.py:9: def Serialize(self, f,
indent=4):
On 2014/07/22 15:34:25, nednguyen wrote:
> I don't like this indent param in the Serialize() method of TimelineData
> abstract interface. This would make no sense if the underlined data is not
> json-compatible.
> 
> Can we simplify the whole situation by always indenting the json file with
> indent=4?

I'm okay with always indent=4, but I'm worried that it would make dumped json
files very large. Isn't it a problem for you and other users?

[nednguyen, 2014-07-23 13:52:59]

On 2014/07/23 04:59:06, Dai Mikurube wrote:
>
https://codereview.chromium.org/315393004/diff/80001/tools/telemetry/telemetr...
> File tools/telemetry/telemetry/timeline/timeline_data.py (right):
> 
>
https://codereview.chromium.org/315393004/diff/80001/tools/telemetry/telemetr...
> tools/telemetry/telemetry/timeline/timeline_data.py:9: def Serialize(self, f,
> indent=4):
> On 2014/07/22 15:34:25, nednguyen wrote:
> > I don't like this indent param in the Serialize() method of TimelineData
> > abstract interface. This would make no sense if the underlined data is not
> > json-compatible.
> > 
> > Can we simplify the whole situation by always indenting the json file with
> > indent=4?
> 
> I'm okay with always indent=4, but I'm worried that it would make dumped json
> files very large. Isn't it a problem for you and other users?
Most people still need to prettify the json file to analyze it anyway, so let
just go ahead and do it. If someone complains, we can always add a method like
to SerializeAsJson(f, indent) to json-compatible timelinedata classes.

[Dai Mikurube (NOT FULLTIME), 2014-08-12 08:26:10]

Sorry that I forgot it. Made it always indent=4.

[nednguyen, 2014-08-12 15:24:36]

On 2014/08/12 08:26:10, Dai Mikurube wrote:
> Sorry that I forgot it. Made it always indent=4.

lgtm again. thanks

[nednguyen, 2014-08-12 15:39:37]

The CQ bit was checked by nednguyen@google.com

[commit-bot: I haz the power, 2014-08-12 15:40:54]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dmikurube@chromium.org/315393004/100001

[commit-bot: I haz the power, 2014-08-12 18:24:29]

Change committed as 289028