Fix histogram timer to generate ProfViz compatible log.

Fix histogram timer to generate ProfViz compatible log.

After histrogram timer added time recaling functionality,
some events, e.g. parse, histogram timer generates event log ending with 'MicroSeconds'.

Since ProfViz can't recorgnize it, this patch cuts off 'MicroSeconds' postfix.

R=vogelheim@chromium.org, yangguo@chromium.org
BUG=chromium:
LOG=N

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

[Yang, 2016-03-08 10:17:19]

On 2016/03/08 10:12:20, Myeong-bo Shim wrote:

Thanks for catching this!

Doing a string operation on every logging entry seems pretty expensive. Can we
adapt the post-processing script to recognize the "MicroSeconds" postfix?

[Myeong-bo Shim, 2016-03-08 10:22:37]

On 2016/03/08 10:17:19, Yang wrote:
> On 2016/03/08 10:12:20, Myeong-bo Shim wrote:
> 
> Thanks for catching this!
> 
> Doing a string operation on every logging entry seems pretty expensive. Can we
> adapt the post-processing script to recognize the "MicroSeconds" postfix?

Sounds good that I catched right thing.

BTW, I want to clear your suggestion.
Do you mean fixing ProfViz or testing tools?

[Yang, 2016-03-08 12:33:54]

On 2016/03/08 10:22:37, Myeong-bo Shim wrote:
> On 2016/03/08 10:17:19, Yang wrote:
> > On 2016/03/08 10:12:20, Myeong-bo Shim wrote:
> > 
> > Thanks for catching this!
> > 
> > Doing a string operation on every logging entry seems pretty expensive. Can
we
> > adapt the post-processing script to recognize the "MicroSeconds" postfix?
> 
> Sounds good that I catched right thing.
> 
> BTW, I want to clear your suggestion.
> Do you mean fixing ProfViz or testing tools?

You can find the list of events that profviz knows about here:
https://code.google.com/p/chromium/codesearch#chromium/src/v8/tools/profviz/c....
At the use site of this, you could look for the "MicroSeconds" suffix, remove
it, and scale the number down accordingly.

If you can't figure out how, please just file a bug and assign it to me :)

[Myeong-bo Shim, 2016-03-09 07:49:34]

On 2016/03/08 12:33:54, Yang wrote:
> On 2016/03/08 10:22:37, Myeong-bo Shim wrote:
> > On 2016/03/08 10:17:19, Yang wrote:
> > > On 2016/03/08 10:12:20, Myeong-bo Shim wrote:
> > > 
> > > Thanks for catching this!
> > > 
> > > Doing a string operation on every logging entry seems pretty expensive.
Can
> we
> > > adapt the post-processing script to recognize the "MicroSeconds" postfix?
> > 
> > Sounds good that I catched right thing.
> > 
> > BTW, I want to clear your suggestion.
> > Do you mean fixing ProfViz or testing tools?
> 
> You can find the list of events that profviz knows about here:
>
https://code.google.com/p/chromium/codesearch#chromium/src/v8/tools/profviz/c....
> At the use site of this, you could look for the "MicroSeconds" suffix, remove
> it, and scale the number down accordingly.
> 
> If you can't figure out how, please just file a bug and assign it to me :)

I fixed ProfViz and new patch set is ready to be uploaded.
However, company network has a problem in upload whose error message as follow.

Access to https://codereview.chromium.org is denied (server returned HTTP 403).

I will try to fix it, first. If it cannot be fixed soon, I will assign it to
you.

[Myeong-bo Shim, 2016-03-11 05:24:56]

On 2016/03/09 07:49:34, Myeong-bo Shim wrote:
> On 2016/03/08 12:33:54, Yang wrote:
> > On 2016/03/08 10:22:37, Myeong-bo Shim wrote:
> > > On 2016/03/08 10:17:19, Yang wrote:
> > > > On 2016/03/08 10:12:20, Myeong-bo Shim wrote:
> > > > 
> > > > Thanks for catching this!
> > > > 
> > > > Doing a string operation on every logging entry seems pretty expensive.
> Can
> > we
> > > > adapt the post-processing script to recognize the "MicroSeconds"
postfix?
> > > 
> > > Sounds good that I catched right thing.
> > > 
> > > BTW, I want to clear your suggestion.
> > > Do you mean fixing ProfViz or testing tools?
> > 
> > You can find the list of events that profviz knows about here:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/v8/tools/profviz/c....
> > At the use site of this, you could look for the "MicroSeconds" suffix,
remove
> > it, and scale the number down accordingly.
> > 
> > If you can't figure out how, please just file a bug and assign it to me :)
> 
> I fixed ProfViz and new patch set is ready to be uploaded.
> However, company network has a problem in upload whose error message as
follow.
> 
> Access to https://codereview.chromium.org is denied (server returned HTTP
403).
> 
> I will try to fix it, first. If it cannot be fixed soon, I will assign it to
> you.

Patch uploaded. Please, take a look at it.

[Yang, 2016-03-11 08:08:39]

Thanks for the fix! Can you also update the comments?

https://codereview.chromium.org/1771293002/diff/20001/tools/profviz/composer.js
File tools/profviz/composer.js (right):

https://codereview.chromium.org/1771293002/diff/20001/tools/profviz/composer....
tools/profviz/composer.js:52: var kTickHalfDuration = 0.5;      // Duration of
half a tick in ms.
Can you update this comment and elsewhere to microseconds? ("us" instead of
"ms".

https://codereview.chromium.org/1771293002/diff/20001/tools/profviz/composer....
tools/profviz/composer.js:81: this.start = start;  // In milliseconds.
microseconds here as well, and below.

https://codereview.chromium.org/1771293002/diff/20001/tools/profviz/composer....
tools/profviz/composer.js:392: output("set xlabel \"execution time in ms\"");
"execution time in microseconds" please.

https://codereview.chromium.org/1771293002/diff/20001/tools/profviz/composer....
tools/profviz/composer.js:530: var label_content = (pause.duration() | 0) + "
ms";
" {/Symbol m}s" instead of " ms"

[Myeong-bo Shim, 2016-03-11 09:03:12]

On 2016/03/11 08:08:39, Yang wrote:
> Thanks for the fix! Can you also update the comments?
> 
>
https://codereview.chromium.org/1771293002/diff/20001/tools/profviz/composer.js
> File tools/profviz/composer.js (right):
> 
>
https://codereview.chromium.org/1771293002/diff/20001/tools/profviz/composer....
> tools/profviz/composer.js:52: var kTickHalfDuration = 0.5;      // Duration of
> half a tick in ms.
> Can you update this comment and elsewhere to microseconds? ("us" instead of
> "ms".
> 
>
https://codereview.chromium.org/1771293002/diff/20001/tools/profviz/composer....
> tools/profviz/composer.js:81: this.start = start;  // In milliseconds.
> microseconds here as well, and below.
> 
>
https://codereview.chromium.org/1771293002/diff/20001/tools/profviz/composer....
> tools/profviz/composer.js:392: output("set xlabel \"execution time in ms\"");
> "execution time in microseconds" please.
> 
>
https://codereview.chromium.org/1771293002/diff/20001/tools/profviz/composer....
> tools/profviz/composer.js:530: var label_content = (pause.duration() | 0) + "
> ms";
> " {/Symbol m}s" instead of " ms"

Time in log is in us but when ProfViz read log file, it converts time-stamp to
ms using parseTimeStamp.
Therefore I think it is better to leave in ms.
How do you think?

[Yang, 2016-03-11 09:43:32]

On 2016/03/11 09:03:12, Myeong-bo Shim wrote:
> On 2016/03/11 08:08:39, Yang wrote:
> > Thanks for the fix! Can you also update the comments?
> > 
> >
>
https://codereview.chromium.org/1771293002/diff/20001/tools/profviz/composer.js
> > File tools/profviz/composer.js (right):
> > 
> >
>
https://codereview.chromium.org/1771293002/diff/20001/tools/profviz/composer....
> > tools/profviz/composer.js:52: var kTickHalfDuration = 0.5;      // Duration
of
> > half a tick in ms.
> > Can you update this comment and elsewhere to microseconds? ("us" instead of
> > "ms".
> > 
> >
>
https://codereview.chromium.org/1771293002/diff/20001/tools/profviz/composer....
> > tools/profviz/composer.js:81: this.start = start;  // In milliseconds.
> > microseconds here as well, and below.
> > 
> >
>
https://codereview.chromium.org/1771293002/diff/20001/tools/profviz/composer....
> > tools/profviz/composer.js:392: output("set xlabel \"execution time in
ms\"");
> > "execution time in microseconds" please.
> > 
> >
>
https://codereview.chromium.org/1771293002/diff/20001/tools/profviz/composer....
> > tools/profviz/composer.js:530: var label_content = (pause.duration() | 0) +
"
> > ms";
> > " {/Symbol m}s" instead of " ms"
> 
> Time in log is in us but when ProfViz read log file, it converts time-stamp to
> ms using parseTimeStamp.
> Therefore I think it is better to leave in ms.
> How do you think?

You are right. I forgot that part.

LGTM. You can click on the "commit" checkbox to land this CL via commit queue.

[Myeong-bo Shim, 2016-03-11 09:45:12]

The CQ bit was checked by m0609.shim@samsung.com

[commit-bot: I haz the power, 2016-03-11 09:45:20]

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

[commit-bot: I haz the power, 2016-03-11 09:50:31]

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

[commit-bot: I haz the power, 2016-03-11 09:50:32]

Try jobs failed on following builders:
  v8_presubmit on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/12254)

[Yang, 2016-03-11 09:51:42]

Description was changed from

==========
Fix histogram timer to generate ProfViz compatible log.

After histrogram timer added time recaling functionality,
some events, e.g. parse, histogram timer generates event log ending with
'MicroSeconds'.

Since ProfViz can't recorgnize it, this patch cuts off 'MicroSeconds' postfix.

R=vogelheim@chromium.org, yangguo@chromium.org
BUG=chromium:592849
==========

to

==========
Fix histogram timer to generate ProfViz compatible log.

After histrogram timer added time recaling functionality,
some events, e.g. parse, histogram timer generates event log ending with
'MicroSeconds'.

Since ProfViz can't recorgnize it, this patch cuts off 'MicroSeconds' postfix.

R=vogelheim@chromium.org, yangguo@chromium.org
BUG=chromium:
LOG=N
==========

[Yang, 2016-03-11 09:51:46]

The CQ bit was checked by yangguo@chromium.org

[commit-bot: I haz the power, 2016-03-11 09:51:50]

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

[Yang, 2016-03-11 09:52:01]

On 2016/03/11 09:50:32, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   v8_presubmit on tryserver.v8 (JOB_FAILED,
> http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/12254)

I fixed the presubmit issue and retriggered CQ.

[commit-bot: I haz the power, 2016-03-11 10:04:47]

Description was changed from

==========
Fix histogram timer to generate ProfViz compatible log.

After histrogram timer added time recaling functionality,
some events, e.g. parse, histogram timer generates event log ending with
'MicroSeconds'.

Since ProfViz can't recorgnize it, this patch cuts off 'MicroSeconds' postfix.

R=vogelheim@chromium.org, yangguo@chromium.org
BUG=chromium:
LOG=N
==========

to

==========
Fix histogram timer to generate ProfViz compatible log.

After histrogram timer added time recaling functionality,
some events, e.g. parse, histogram timer generates event log ending with
'MicroSeconds'.

Since ProfViz can't recorgnize it, this patch cuts off 'MicroSeconds' postfix.

R=vogelheim@chromium.org, yangguo@chromium.org
BUG=chromium:
LOG=N
==========

[commit-bot: I haz the power, 2016-03-11 10:04:48]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-03-11 10:05:33]

Description was changed from

==========
Fix histogram timer to generate ProfViz compatible log.

After histrogram timer added time recaling functionality,
some events, e.g. parse, histogram timer generates event log ending with
'MicroSeconds'.

Since ProfViz can't recorgnize it, this patch cuts off 'MicroSeconds' postfix.

R=vogelheim@chromium.org, yangguo@chromium.org
BUG=chromium:
LOG=N
==========

to

==========
Fix histogram timer to generate ProfViz compatible log.

After histrogram timer added time recaling functionality,
some events, e.g. parse, histogram timer generates event log ending with
'MicroSeconds'.

Since ProfViz can't recorgnize it, this patch cuts off 'MicroSeconds' postfix.

R=vogelheim@chromium.org, yangguo@chromium.org
BUG=chromium:
LOG=N

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

[commit-bot: I haz the power, 2016-03-11 10:05:34]

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