IonMonkey: Integrate with VTune

RESOLVED DUPLICATE of bug 1332466

Status

()

Core
JavaScript Engine
RESOLVED DUPLICATE of bug 1332466
6 years ago
a year ago

People

(Reporter: dvander, Assigned: dvander)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ion:t])

Attachments

(2 attachments, 7 obsolete attachments)

Created attachment 634715 [details] [diff] [review]
WIP v0

Very hacked up patch. Works, basically, but there are holes in JIT code coverage (like VMWrappers/OOL paths).

This patch also takes out ebp as a general register (if --enable-profiling) and saves it in prologues, so VTune can construct js->js callstacks.
Created attachment 636961 [details] [diff] [review]
wip v1

this version maps VMWrappers (unfortunately, i couldnt grab the function names), and ICs, which show as things like "ic-getprop-native" and map to the file and line the property access occurred on (rather than the place they were potentially inlined).
Attachment #634715 - Attachment is obsolete: true

Comment 2

6 years ago
I have been crashing with the v1 patch, and it looks to be caused by the disabling of the check if the profiler is running. Uncommenting this check appears to fix in the small test case I have run so far: 


+++ b/js/src/ion/IonCaches.cpp
@@ -52,16 +52,41 @@
> 
> #include "vm/Stack.h"
> #include "IonFrames-inl.h"
> 
> using namespace js;
> using namespace js::ion;
> 
> void
>+InstrumentCache(IonCode *code, JSScript *script, jsbytecode *pc, const char *name)
>+{
>+#ifdef MOZ_VTUNE
>+    //if (iJIT_SAMPLING_ON != iJIT_IsProfilingActive())
>+    //    return;

Comment 3

6 years ago
The fix I suggested above turns out to be more of a workaround while there is no collection.

I think the cause is that the JSScript * in the second argument is coming in as NULL, so script->filename is failing.
Whiteboard: [ion:t]

Comment 5

6 years ago
Created attachment 652278 [details] [diff] [review]
wipv1 + Windows support + bugfix

Showing Windows a little love, and fixing a compile error in jsdbgapi.h.

Comment 6

6 years ago
Created attachment 652493 [details] [diff] [review]
wipv1 + Windows support + bugfix

Minor change to fix VTune crash on finalization.
Attachment #652278 - Attachment is obsolete: true

Comment 7

6 years ago
Created attachment 652544 [details] [diff] [review]
wipv1 + Windows support + bugfix
Attachment #652493 - Attachment is obsolete: true
Created attachment 700855 [details] [diff] [review]
WIP v2

Entirely new patch based on bug 785234, includes some of Joe's fixes. This one is factored with the intent of being checked in, and tries to be smarter about reporting inlined function calls. I still have to get out-of-line chunks and ICs working.
Attachment #636961 - Attachment is obsolete: true
Attachment #646789 - Attachment is obsolete: true
Created attachment 700856 [details] [diff] [review]
WIP v2 actual patch

Can't actually test yet, but it builds.
Attachment #700855 - Attachment is obsolete: true
Created attachment 700871 [details] [diff] [review]
more generalized patch

This version removes the VTune dependency for mapping line numbers, so it should be usable by SPS and OProfile as well. It also adds OOL support.

TODO: testing, ICs, x64.
Attachment #700856 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1332466
You need to log in before you can comment on or make changes to this bug.