Adding src_file_name:line_number into perf log entries for compiled JS functi...

Adding src_file_name:line_number into perf log entries for compiled JS functions.

Thus, instead of the following profiler records:
   1.5%    1.5%   LazyCompile: <anonymous>
we'll now have these:
   1.5%    1.5%   LazyCompile: <anonymous> richards.js:309

Basically, I translated two functions from messages.js into C++.
In the next CL I will update messages.js to use added native functions.


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

[Søren Thygesen Gjesse, 2009-02-02 14:40:45]

Mostly nits.

http://codereview.chromium.org/19537/diff/1/8
File src/factory.cc (right):

http://codereview.chromium.org/19537/diff/1/8#newcode163
Line 163: script->set_line_ends(*(Factory::NewFixedArray(0)));
Use Heap::undefined_value() to mark not initialized.

http://codereview.chromium.org/19537/diff/1/7
File src/log.cc (right):

http://codereview.chromium.org/19537/diff/1/7#newcode589
Line 589: 
Please use two empty lines between methods.

http://codereview.chromium.org/19537/diff/1/7#newcode604
Line 604: 
Please use two empty lines between methods.

http://codereview.chromium.org/19537/diff/1/7#newcode616
Line 616: 
Please use two empty lines between methods.

http://codereview.chromium.org/19537/diff/1/6
File src/objects-inl.h (right):

http://codereview.chromium.org/19537/diff/1/6#newcode1992
Line 1992: ACCESSORS(Script, line_ends, FixedArray, kLineEndsOffset)
You should use Object here to allow it to be initialized to undefined.

http://codereview.chromium.org/19537/diff/1/3
File src/objects.cc (right):

http://codereview.chromium.org/19537/diff/1/3#newcode6780
Line 6780: if (line_ends()->length() > 0) return;
Check for undefined using IsUndefined(). Add an ASSERT  IsFixedArray (or
IsJSArray - see below) before returning.

http://codereview.chromium.org/19537/diff/1/3#newcode6783
Line 6783: SmartPointer<uc16> data = src->ToWideCString();
ToWideCString is only for debugging and logging purposes. Try to look at how
indexOf is implemented in Runtime_StringIndexOf. Perhaps using
Runtime::StringMatch with a one character pattern might work.
Runtime::StringMatch is used for simple string search in the RegExp
implementation.

http://codereview.chromium.org/19537/diff/1/3#newcode6800
Line 6800:
set_line_ends(FixedArray::cast(line_ends()->AddKeysFromJSArray(*array)));
When exposing this to JavaScript you will probably end up storing the JSArray
instead of the FixedArray, but you can wait with that change until the next
changelist.

http://codereview.chromium.org/19537/diff/1/3#newcode6813
Line 6813: for (int i = 1; i < line_ends()->length(); ++i) {
Issue 213 (http://code.google.com/p/v8/issues/detail?id=213) suggests we change
this to binary search. You can look at that in a separate changelist.

http://codereview.chromium.org/19537/diff/1/5
File src/objects.h (right):

http://codereview.chromium.org/19537/diff/1/5#newcode2541
Line 2541: DECL_ACCESSORS(line_ends, FixedArray)
FixedArray -> Object

[Mikhail Naganov, 2009-02-03 09:23:09]

http://codereview.chromium.org/19537/diff/1/8
File src/factory.cc (right):

http://codereview.chromium.org/19537/diff/1/8#newcode163
Line 163: script->set_line_ends(*(Factory::NewFixedArray(0)));
On 2009/02/02 14:40:45, Søren Gjesse wrote:
> Use Heap::undefined_value() to mark not initialized.

Done.

http://codereview.chromium.org/19537/diff/1/7
File src/log.cc (right):

http://codereview.chromium.org/19537/diff/1/7#newcode589
Line 589: 
On 2009/02/02 14:40:45, Søren Gjesse wrote:
> Please use two empty lines between methods.

Done.

http://codereview.chromium.org/19537/diff/1/7#newcode604
Line 604: 
On 2009/02/02 14:40:45, Søren Gjesse wrote:
> Please use two empty lines between methods.

Done.

http://codereview.chromium.org/19537/diff/1/7#newcode616
Line 616: 
On 2009/02/02 14:40:45, Søren Gjesse wrote:
> Please use two empty lines between methods.

Done.

http://codereview.chromium.org/19537/diff/1/6
File src/objects-inl.h (right):

http://codereview.chromium.org/19537/diff/1/6#newcode1992
Line 1992: ACCESSORS(Script, line_ends, FixedArray, kLineEndsOffset)
On 2009/02/02 14:40:45, Søren Gjesse wrote:
> You should use Object here to allow it to be initialized to undefined.

Done.

http://codereview.chromium.org/19537/diff/1/3
File src/objects.cc (right):

http://codereview.chromium.org/19537/diff/1/3#newcode6780
Line 6780: if (line_ends()->length() > 0) return;
On 2009/02/02 14:40:45, Søren Gjesse wrote:
> Check for undefined using IsUndefined(). Add an ASSERT  IsFixedArray (or
> IsJSArray - see below) before returning.
> 

Done.

http://codereview.chromium.org/19537/diff/1/3#newcode6783
Line 6783: SmartPointer<uc16> data = src->ToWideCString();
On 2009/02/02 14:40:45, Søren Gjesse wrote:
> ToWideCString is only for debugging and logging purposes. Try to look at how
> indexOf is implemented in Runtime_StringIndexOf. Perhaps using
> Runtime::StringMatch with a one character pattern might work.
> Runtime::StringMatch is used for simple string search in the RegExp
> implementation.
> 

Done.

http://codereview.chromium.org/19537/diff/1/3#newcode6800
Line 6800:
set_line_ends(FixedArray::cast(line_ends()->AddKeysFromJSArray(*array)));
On 2009/02/02 14:40:45, Søren Gjesse wrote:
> When exposing this to JavaScript you will probably end up storing the JSArray
> instead of the FixedArray, but you can wait with that change until the next
> changelist.

Using FixedArray, because when I try to use a JSArray, a leak in global objects
is detected by a test.

http://codereview.chromium.org/19537/diff/1/3#newcode6813
Line 6813: for (int i = 1; i < line_ends()->length(); ++i) {
On 2009/02/02 14:40:45, Søren Gjesse wrote:
> Issue 213 (http://code.google.com/p/v8/issues/detail?id=213) suggests we
change
> this to binary search. You can look at that in a separate changelist.

Yes, I'm for a separate CL.

http://codereview.chromium.org/19537/diff/1/5
File src/objects.h (right):

http://codereview.chromium.org/19537/diff/1/5#newcode2541
Line 2541: DECL_ACCESSORS(line_ends, FixedArray)
On 2009/02/02 14:40:45, Søren Gjesse wrote:
> FixedArray -> Object

Done.

[Søren Thygesen Gjesse, 2009-02-03 09:44:14]

LGTM

One thing you might look into in a new CL is traversing the string twice to
first count the number of lines and then allocate a FixedArray of the right size
and filling it on second traversal. Using a JSArray will result in element
copying each time the underlying FixedArray is grown, and the AddKeysFromJSArray
also copies all the elements. I don't know whether the difference would be
measurable. See NewElementsCapacity for the growth factor. Also at size
kMaxFastElementsLength (5000) the JSArray will move to slow (dictionary) case.

[Mikhail Naganov, 2009-02-03 10:49:17]

On 2009/02/03 09:44:14, Søren Gjesse wrote:
> LGTM
> 
> One thing you might look into in a new CL is traversing the string twice to
> first count the number of lines and then allocate a FixedArray of the right
size
> and filling it on second traversal. Using a JSArray will result in element
> copying each time the underlying FixedArray is grown, and the
AddKeysFromJSArray
> also copies all the elements. I don't know whether the difference would be
> measurable. See NewElementsCapacity for the growth factor. Also at size
> kMaxFastElementsLength (5000) the JSArray will move to slow (dictionary) case.

Thanks! Yes, I considered using two passes but thought of it as of premature
optimization.