Patch updated according to Michail's comments. Working on unit tests now.
http://codereview.chromium.org/1694011/diff/9001/10002 File include/v8.h (right): http://codereview.chromium.org/1694011/diff/9001/10002#newcode772 include/v8.h:772: * This method will return 0 if it is unable to retrieve the line number, On 2010/04/30 10:01:22, Michail Naganov wrote:
It's better to use a named constant for the "unable to retrieve" case.
E.g. look
at the "Function" class.
Done. http://codereview.chromium.org/1694011/diff/9001/10002#newcode781 include/v8.h:781: * This method will return 0 if it is unable to retrieve the line number, On 2010/04/30 10:01:22, Michail Naganov wrote:
The same here.
Done. http://codereview.chromium.org/1694011/diff/9001/10003 File src/top.cc (right): http://codereview.chromium.org/1694011/diff/9001/10003#newcode380 src/top.cc:380: Handle<JSArray> stackTrace = Factory::NewJSArray(frame_limit); On 2010/04/30 10:01:22, Michail Naganov wrote:
JSArray can be a bit slow because of dynamic expansion support. An
alternative
approach is to count frames number first, then allocate a fixed array.
I thought the dynamic expansion code path will only trigger if I exceed the initial size that I create it with. I pass in frame_limit which is guaranteed to be >= frames_seen. The reason why I use JSArray is that I want to be able to easily return the StackTrace as a v8::Array for external consumption in JavaScript. Also, If you notice, I only add elements to the JSArray via its elements(), which is a FixedArray :). http://codereview.chromium.org/1694011/diff/9001/10003#newcode408 src/top.cc:408: CStrVector(kColumnKeyData)); On 2010/04/30 10:01:22, Michail Naganov wrote:
You could pre-allocate those strings as symbols (see SYMBOL_LIST in
heap.h) to
avoid creating them every time. Or, at least, move their allocation
out of the
loop.
I will move them out of the loop. Should I guard each key symbol allocation with a flag guard? I think allocating them all should be fine since there are not very many. The cost of the extra branches might outweigh the allocation cost. http://codereview.chromium.org/1694011/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
