Add source map support to tick processor.

Add source map support to tick processor.

Added a console parameter for source map to the tick processor.
The tickprocesspor reads in the source maps and uses it to output the original filename, line number and column in the profile.
Modified d8 to output column numbers into the log, since this is needed to do source mapping.

R=jkummerow@chromium.org

Committed: http://code.google.com/p/v8/source/detail?r=16307

[Daniel Kurka, 2013-08-23 11:31:32]

Can you please take a look

[Jakob Kummerow, 2013-08-23 13:08:35]

Looks pretty good, except for formatting.

https://codereview.chromium.org/22897021/diff/3001/tools/tickprocessor.js
File tools/tickprocessor.js (left):

https://codereview.chromium.org/22897021/diff/3001/tools/tickprocessor.js#old...
tools/tickprocessor.js:33: 
nit: leave this line in please.

https://codereview.chromium.org/22897021/diff/3001/tools/tickprocessor.js
File tools/tickprocessor.js (right):

https://codereview.chromium.org/22897021/diff/3001/tools/tickprocessor.js#new...
tools/tickprocessor.js:552: return null;
nit: indentation

https://codereview.chromium.org/22897021/diff/3001/tools/tickprocessor.js#new...
tools/tickprocessor.js:866: '--sourceMap': ['sourceMap', null,
For consistency with other arg names, let's call this --source-map.

https://codereview.chromium.org/22897021/diff/3001/tools/tickprocessor.js#new...
tools/tickprocessor.js:867: 'Specify the source map that should be used for
output']
nit: indentation

https://codereview.chromium.org/22897021/diff/3001/tools/tickprocessor.js#new...
tools/tickprocessor.js:955: * Implements Source Map V3 model. See
http://code.google.com/p/closure-compiler/wiki/SourceMaps
This section looks a lot like
http://src.chromium.org/viewvc/blink/trunk/Source/devtools/front_end/SourceMa....
How about a comment stating the code's origin?

I'd like to suggest that we simply check in an exact copy of that file, to make
any future updates trivial. But I see that we need several modifications to make
things work in d8.

Since we need substantial modifications anyway, the code should adhere to V8
style: 2-space indentation, 80-column lines, opening '{' on the previous line.
(Compare with existing code.)
Most of the type annotations don't provide value for a human reader, so they can
be removed. Feel free to keep those that convey non-obvious information.

[Daniel Kurka, 2013-08-23 13:31:43]

https://codereview.chromium.org/22897021/diff/3001/tools/tickprocessor.js
File tools/tickprocessor.js (left):

https://codereview.chromium.org/22897021/diff/3001/tools/tickprocessor.js#old...
tools/tickprocessor.js:33: 
On 2013/08/23 13:08:35, Jakob wrote:
> nit: leave this line in please.

Done.

https://codereview.chromium.org/22897021/diff/3001/tools/tickprocessor.js
File tools/tickprocessor.js (right):

https://codereview.chromium.org/22897021/diff/3001/tools/tickprocessor.js#new...
tools/tickprocessor.js:552: return null;
On 2013/08/23 13:08:35, Jakob wrote:
> nit: indentation

Done.

https://codereview.chromium.org/22897021/diff/3001/tools/tickprocessor.js#new...
tools/tickprocessor.js:866: '--sourceMap': ['sourceMap', null,
On 2013/08/23 13:08:35, Jakob wrote:
> For consistency with other arg names, let's call this --source-map.

Done.

https://codereview.chromium.org/22897021/diff/3001/tools/tickprocessor.js#new...
tools/tickprocessor.js:867: 'Specify the source map that should be used for
output']
On 2013/08/23 13:08:35, Jakob wrote:
> nit: indentation

Done.

https://codereview.chromium.org/22897021/diff/3001/tools/tickprocessor.js#new...
tools/tickprocessor.js:955: * Implements Source Map V3 model. See
http://code.google.com/p/closure-compiler/wiki/SourceMaps
Actually we only need a modified load method

The rest of the file does not have any modifications, so to make future updates
easy I think we should leave it as is.

I don't think we should check in a complete copy, how about a comment with the
original commit id this was taken from?




On 2013/08/23 13:08:35, Jakob wrote:
> This section looks a lot like
>
http://src.chromium.org/viewvc/blink/trunk/Source/devtools/front_end/SourceMa....
> How about a comment stating the code's origin?
> 
> I'd like to suggest that we simply check in an exact copy of that file, to
make
> any future updates trivial. But I see that we need several modifications to
make
> things work in d8.
> 
> Since we need substantial modifications anyway, the code should adhere to V8
> style: 2-space indentation, 80-column lines, opening '{' on the previous line.
> (Compare with existing code.)
> Most of the type annotations don't provide value for a human reader, so they
can
> be removed. Feel free to keep those that convey non-obvious information.

[Daniel Kurka, 2013-08-23 14:26:41]

I had to skip the presubmit check since putting a comment over the original file
content of SourceMap.js violates the rule of copyright statement first. Let me
know if you think its fine that way.

[Jakob Kummerow, 2013-08-23 15:13:37]

LGTM if you fix the last few nits.

To appease the linter, I'd suggest to move the added lines in SourceMap.js after
the copyright boilerplate.

https://codereview.chromium.org/22897021/diff/15001/tools/tickprocessor-drive...
File tools/tickprocessor-driver.js (right):

https://codereview.chromium.org/22897021/diff/15001/tools/tickprocessor-drive...
tools/tickprocessor-driver.js:41: // pull it into our name space
nit: Comments should start with a capital letter and end with appropriate
punctuation.
(again below)

[Daniel Kurka, 2013-08-23 15:47:37]

https://codereview.chromium.org/22897021/diff/15001/tools/tickprocessor-drive...
File tools/tickprocessor-driver.js (right):

https://codereview.chromium.org/22897021/diff/15001/tools/tickprocessor-drive...
tools/tickprocessor-driver.js:41: // pull it into our name space
On 2013/08/23 15:13:37, Jakob wrote:
> nit: Comments should start with a capital letter and end with appropriate
> punctuation.
> (again below)

Done.

[Daniel Kurka, 2013-08-23 15:56:28]

Added a proper v8 copyright header to SourceMap.js (this is a different format
from blink) and moved copy note and everything below that. Should be simple to
update in the future.

[Jakob Kummerow, 2013-08-23 15:57:29]

LGTM, I'll land this for you.

[Daniel Kurka, 2013-08-23 16:09:07]

On 2013/08/23 15:57:29, Jakob wrote:
> LGTM, I'll land this for you.

Thank you Jakob for your time!

[Jakob Kummerow, 2013-08-23 17:21:08]

Committed patchset #5 manually as r16307.