Add support for dumping code in jitdump file format

Add support for dumping code in jitdump file format

R=iposva@google.com

Committed: https://code.google.com/p/dart/source/detail?r=33098

[Cutch, 2013-09-18 17:25:50]

Not ready for review yet. Need to iterate with Stephane.

[Cutch, 2013-12-11 20:01:32]

PTAL

[Cutch, 2014-01-31 19:19:10]

*bump*

[Ivan Posva, 2014-02-06 00:33:26]

-Ivan

https://codereview.chromium.org/23437046/diff/170001/runtime/vm/os_linux.cc
File runtime/vm/os_linux.cc (right):

https://codereview.chromium.org/23437046/diff/170001/runtime/vm/os_linux.cc#n...
runtime/vm/os_linux.cc:283: JitdumpCodeObserver() {
ASSERT that the relevant flag is set.

https://codereview.chromium.org/23437046/diff/170001/runtime/vm/os_linux.cc#n...
runtime/vm/os_linux.cc:291: }
Don't you want to check the file_close callback here as well?

https://codereview.chromium.org/23437046/diff/170001/runtime/vm/os_linux.cc#n...
runtime/vm/os_linux.cc:292: const char* format = "/tmp/jit-%ld.dump";
// Comments please. What are we going to access here?

https://codereview.chromium.org/23437046/diff/170001/runtime/vm/os_linux.cc#n...
runtime/vm/os_linux.cc:303: printf("Using /dev/trace_clock as timesource
(%x).\n", clock_id_);
Do you really want to print this every time?

https://codereview.chromium.org/23437046/diff/170001/runtime/vm/os_linux.cc#n...
runtime/vm/os_linux.cc:391: #else
Generally we want to get compilation errors if we can. In this case we can:

#else
#error Unknown architecture.
#endif

https://codereview.chromium.org/23437046/diff/170001/runtime/vm/os_linux.cc#n...
runtime/vm/os_linux.cc:397: return syscall(SYS_gettid);
Why do we have to go through syscall here? Is there no way to call this
directly? At least please add a comment explaining what is happening here.

https://codereview.chromium.org/23437046/diff/170001/runtime/vm/os_linux.cc#n...
runtime/vm/os_linux.cc:450: (*file_write)(&code_load, sizeof(code_load),
out_file_);
Multiple isolates will collide here.

Please verify other observers. Thanks!

[Cutch, 2014-02-13 19:16:57]

PTAL.

https://codereview.chromium.org/23437046/diff/170001/runtime/vm/os_linux.cc
File runtime/vm/os_linux.cc (right):

https://codereview.chromium.org/23437046/diff/170001/runtime/vm/os_linux.cc#n...
runtime/vm/os_linux.cc:283: JitdumpCodeObserver() {
On 2014/02/06 00:33:26, Ivan Posva wrote:
> ASSERT that the relevant flag is set.

Done.

https://codereview.chromium.org/23437046/diff/170001/runtime/vm/os_linux.cc#n...
runtime/vm/os_linux.cc:291: }
On 2014/02/06 00:33:26, Ivan Posva wrote:
> Don't you want to check the file_close callback here as well?

Done.

https://codereview.chromium.org/23437046/diff/170001/runtime/vm/os_linux.cc#n...
runtime/vm/os_linux.cc:292: const char* format = "/tmp/jit-%ld.dump";
On 2014/02/06 00:33:26, Ivan Posva wrote:
> // Comments please. What are we going to access here?

Done.

https://codereview.chromium.org/23437046/diff/170001/runtime/vm/os_linux.cc#n...
runtime/vm/os_linux.cc:303: printf("Using /dev/trace_clock as timesource
(%x).\n", clock_id_);
On 2014/02/06 00:33:26, Ivan Posva wrote:
> Do you really want to print this every time?
Removed.

https://codereview.chromium.org/23437046/diff/170001/runtime/vm/os_linux.cc#n...
runtime/vm/os_linux.cc:391: #else
On 2014/02/06 00:33:26, Ivan Posva wrote:
> Generally we want to get compilation errors if we can. In this case we can:
> 
> #else
> #error Unknown architecture.
> #endif

Done.

https://codereview.chromium.org/23437046/diff/170001/runtime/vm/os_linux.cc#n...
runtime/vm/os_linux.cc:397: return syscall(SYS_gettid);
On 2014/02/06 00:33:26, Ivan Posva wrote:
> Why do we have to go through syscall here? Is there no way to call this
> directly? At least please add a comment explaining what is happening here.

libc doesn't wrap Linux-specific gettid, i've added a note.

https://codereview.chromium.org/23437046/diff/170001/runtime/vm/os_linux.cc#n...
runtime/vm/os_linux.cc:450: (*file_write)(&code_load, sizeof(code_load),
out_file_);
On 2014/02/06 00:33:26, Ivan Posva wrote:
> Multiple isolates will collide here.
> 
> Please verify other observers. Thanks!

Done.

[Ivan Posva, 2014-02-25 08:09:04]

LGTMwC

-Ivan

https://codereview.chromium.org/23437046/diff/230001/runtime/vm/code_observer...
File runtime/vm/code_observers.cc (right):

https://codereview.chromium.org/23437046/diff/230001/runtime/vm/code_observer...
runtime/vm/code_observers.cc:5: #include "platform/thread.h"
The directly associated header is always included first. Please move this below
"vm/code_observers.h"

https://codereview.chromium.org/23437046/diff/230001/runtime/vm/os_linux.cc
File runtime/vm/os_linux.cc (right):

https://codereview.chromium.org/23437046/diff/230001/runtime/vm/os_linux.cc#n...
runtime/vm/os_linux.cc:306: const char* format = "/tmp/jit-%ld.dump";
We have the macro Pd for intptr_t sized %d formats (see for example
PerfCodeObserver).

https://codereview.chromium.org/23437046/diff/230001/runtime/vm/os_linux.cc#n...
runtime/vm/os_linux.cc:414: // libc does wrap the Linux-specific gettid system
call.
does or doesn't?

[Cutch, 2014-02-26 22:28:28]

https://codereview.chromium.org/23437046/diff/230001/runtime/vm/code_observer...
File runtime/vm/code_observers.cc (right):

https://codereview.chromium.org/23437046/diff/230001/runtime/vm/code_observer...
runtime/vm/code_observers.cc:5: #include "platform/thread.h"
On 2014/02/25 08:09:04, Ivan Posva wrote:
> The directly associated header is always included first. Please move this
below
> "vm/code_observers.h"

Done.

https://codereview.chromium.org/23437046/diff/230001/runtime/vm/os_linux.cc
File runtime/vm/os_linux.cc (right):

https://codereview.chromium.org/23437046/diff/230001/runtime/vm/os_linux.cc#n...
runtime/vm/os_linux.cc:306: const char* format = "/tmp/jit-%ld.dump";
On 2014/02/25 08:09:04, Ivan Posva wrote:
> We have the macro Pd for intptr_t sized %d formats (see for example
> PerfCodeObserver).

Done.

https://codereview.chromium.org/23437046/diff/230001/runtime/vm/os_linux.cc#n...
runtime/vm/os_linux.cc:414: // libc does wrap the Linux-specific gettid system
call.
On 2014/02/25 08:09:04, Ivan Posva wrote:
> does or doesn't?

Done.

[Cutch, 2014-02-26 22:30:29]

Committed patchset #16 manually as r33098 (presubmit successful).