Regarding GDBJIT support, that's slightly more broken than it was. I had
intended to attempt to fix it in a second CL, to which end I kept the
CodeEvents
interface pretty much identical to the GDBJIT interface - with the
exception of
the redundant tag parameter.
I can tack those changes onto this CL if you prefer?
http://codereview.chromium.org/10795074/diff/1018/src/builtins.cc
File src/builtins.cc (right):
http://codereview.chromium.org/10795074/diff/1018/src/builtins.cc#newcode1646
src/builtins.cc:1646: JIT_CODE_EVENT(AddCode(functions[i].s_name,
On 2012/07/25 13:50:42, danno wrote:
Why do you remove the code event type? It seems that this even if you
don't use
this, it will be useful for other systems, like GDB.
I discussed this with @vegorov at some length, and he feels as I do that
these types don't have much value. I've replaced them with labels based
on code->kind(), which gives you pretty much the same information.
http://codereview.chromium.org/10795074/diff/1018/src/code-events.cc
File src/code-events.cc (right):
http://codereview.chromium.org/10795074/diff/1018/src/code-events.cc#newcode94
src/code-events.cc:94: void CodeEvents::AddCode(Code* code) {
On 2012/07/25 13:50:42, danno wrote:
Can you handle this with default parameters with a single AddCode API?
I imagine I can, do you mind if I fold that into a second CL with a fix
to the GDBJIT code?
http://codereview.chromium.org/10795074/diff/1018/src/code-events.h
File src/code-events.h (right):
http://codereview.chromium.org/10795074/diff/1018/src/code-events.h#newcode53
src/code-events.h:53: static void AddCode(Handle<String> name,
On 2012/07/25 13:50:42, danno wrote:
I find it a little odd that this interface takes both handles and raw
pointers.
You probably should be uniform in the API, using all Handles or all
pointers.
Since you call this from pointer-based code, you probably should keep
everything
pointer-based, since calling handle-based APIs from pointer-based code
is
generally not a good idea.
The interface here is substantially the same as the GDBJIT interface
(with the exception of removing the tag parameter). I did this to make
it easy to tack the GDBJIT code onto the JIT_CODE macros.
http://codereview.chromium.org/10795074/diff/1018/src/code-events.h#newcode55
src/code-events.h:55: Handle<Code> code,
On 2012/07/25 13:50:42, danno wrote:
Order or parameters same? In fact, if both routines are pointer based,
don't you
only need a single API call?
SGTM
http://codereview.chromium.org/10795074/diff/1018/src/compiler.cc
File src/compiler.cc (right):
http://codereview.chromium.org/10795074/diff/1018/src/compiler.cc#newcode496
src/compiler.cc:496:
JIT_CODE_EVENT(AddCode(Handle<String>(String::cast(script->name())),
On 2012/07/25 13:50:42, danno wrote:
I know this is beyond the scope of your change, but is there any way
to unify
this with the PROFILE code events, since that seems to be a bottleneck
for all
code creation?
It really should be possible to do that, though that would probably
require changing or rewriting much of all three mechanisms. There's no
question that there would be maintenance benefits to this, but I don't
feel it's something I can justify taking on for my needs :(.
http://codereview.chromium.org/10795074/diff/1018/src/gdb-jit.cc
File src/gdb-jit.cc (right):
http://codereview.chromium.org/10795074/diff/1018/src/gdb-jit.cc#newcode49
src/gdb-jit.cc:49: #elif !defined(WIN32) // DO NOT SUBMIT
On 2012/07/25 13:50:42, danno wrote:
What is this change for?
Reverted, sorry.
http://codereview.chromium.org/10795074/diff/1018/src/mark-compact.cc
File src/mark-compact.cc (right):
http://codereview.chromium.org/10795074/diff/1018/src/mark-compact.cc#newcode692
src/mark-compact.cc:692: // DO NOT SUBMIT.
On 2012/07/25 13:50:42, danno wrote:
Is this left-over debug code?
This and the other crud was my initial attempt at trying to compiler
under ENABLE_GDB_JIT_INTERFACE.
http://codereview.chromium.org/10795074/diff/4001/include/v8.h
File include/v8.h (right):
http://codereview.chromium.org/10795074/diff/4001/include/v8.h#newcode3256
include/v8.h:3256: * notified each time code is added, moved or removed.
On 2012/07/25 14:22:12, Mikhail Naganov (Chromium) wrote:
There is a problem with code objects that were created before the
handler has
been set. For V8's profiler we walk the heap in order to enumerate
them prior to
hooking on events. Otherwise, the client will be receiving code move
events for
objects he doesn't know anything about.
Or you need to require that the client must set the handler in the
very
beginning.
This is true - for the use case I have in mind, there's no problem
simply requiring that the hook is instated before initializing V8.
Do you have other use cases in mind here?
Would you like me to change the API, the documentation or both, and if
so, how?
http://codereview.chromium.org/10795074/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
