On 2010/04/23 08:30:56, Søren Gjesse wrote:
On 2010/04/22 18:17:54, jaimeyap wrote:
> Requesting feedback. Will cover with appropriate test cases once
consensus
is
> reached on what the API should look like.
Thanks for the feedback. This is my first V8 patch so forgive me if I don't
have
a full grasp of the system :).
First of all it is fine to have an ability to get stack trace information
from
the C++ API. Maybe it should be in the normal API instead of in the debug
API.
Sounds good.
As far as I can see the basic stack traversal is the same as is used in
Runtime_CollectStackTrace, but instead of just storing relevant
information
from
the frame some calculations are done along the way. The main purpose of
Runtime_CollectStackTrace is to be fast at collecting data as the
information
will most likely be discarded.
Yes, however it stops resolving when grabbing the PC. Meaning in order to
resolve the stack trace, you need to do a second full walk of all the frames
after the fact. Probably not a big deal in practice, but definitely
non-optimal.
Also, is Runtime_CollectStackTrace available via some C++ header? I was
under
the impression that is was purely C++/JS glue functionality.
Also you new API will not avoid garbage collection as is allocates new
objects
and calls functions which might allocate objects as well.
I was not aware that GC could run during object allocation. I was under the
impression that GC would only run when invoking/running JavaScript. I see no
calls to CollectGarbage or related functions anywhere along the allocation
stack
in factory.cc or heap.cc for allocation of JSArrays. I also did not observe
any
GC runs during my tests of this new function (tested grabbing stack traces
for
almost every cross from JS into the DOM bindings when running Gmail and
Wave).
Ofcourse, I am probably just getting lost somewhere following the macros :).
I don't think you should mention any specific numbers on the speed of one
API
over another. This may change over time, and each API provides different
levels
of information.
Makes sense. I just wanted some feedback. The comments will most likely be
streamlined after figuring out what the API should look like.
From the API point of wiev I think the information returned should be
presented
as a C++ objects and not just an array of arrays. Internally using an
array is
fine, but I don't think it should be exposed. I think you should be able
to
create objects which "wraps" the arrays generated just like the API
objects
"wraps" internal handles. E.g. the class Message is an interface to an
internal
JS Object, and likewise class StackTrace and class StackFrame could be
interfaces to a FixedArray (no need for a JS Array then).
Local<v8::StackTrace> GetStackTrace(<flags to indicate level of
information>)
class StackTrace {
int GetFrameCount();
Local<v8::StackFrame> GetFrame(int index);
}
class StackFrame {
...
}
This makes sense. I thought I was following precedent of other methods in
v8-debug.h that returned Handles to Value and Context objects. But a C++ API
would be a lot more flexible for the general use case.
I also think that it should be possible to decide what information to get
hold
of using some kind of flags, so that we are not ending up with a fixed API
which
can only return two strings and two numbers for each frame.
Totally in agreement :). I was hoping to do this iteratively so that I could
satisfy WebKit's usecase and then build a more general API. But it should
not be
that much more code to get most of the way the first time around. I think I
will
try wrapping things up in C++ API in my second iteration.
Of cause patches to improve performance of the other ways of working with
the
stack are always welcome.
I think they are limited fundamentally. I first attempted to speed up the
other
ways of grabbing stack traces. The current mechanisms provide clean JS API.
But
this API crosses boundaries between JS and C++ frequently, and allocates
lots of
wrapper JS objects. I tried to make them faster, but was unable to without
building a separate API all together. I got some wins with a separate JS
API,
but then ran into the problem of GC running willy nilly which was a complete
non-starter. As a reference, my "faster" JS API for grabbing stack traces
was
about 5x faster than the current mechanism. The native API in this patch is
about 150x faster than my "faster" JS API and does not seem to trigger GC.
http://codereview.chromium.org/1694011/show
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
