Closed Bug 861440 Opened 7 years ago Closed 7 years ago

OdinMonkey: Add VTune Instrumentation

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: sstangl, Assigned: sstangl)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

This bug is for adding support for VTune profiling as a first step toward comparing the quality of generated Odin code with the corresponding pre-emscripten C++. Given a (sampling) profile of a run of the C++ code, and a (sampling) profile of a run of the JS code, we should be able to isolate any relative performance differences.
Attached patch WIP VTune Support (obsolete) — Splinter Review
This works about as well as the JIT profiling API will let it. I will put the patch up for review once I figure out how to get VTune to do something useful with the data it collects.
Blocks: 863924
There are changes to VTune coming at some point for function unregistration, but we might as well get this patch landed now.
Attachment #738258 - Attachment is obsolete: true
Attachment #746616 - Flags: review?(luke)
Comment on attachment 746616 [details] [diff] [review]
VTune Instrumentation

Review of attachment 746616 [details] [diff] [review]:
-----------------------------------------------------------------

Cool, thanks!

::: js/src/Makefile.in
@@ +706,5 @@
>  endif
>  
>  ifdef MOZ_VTUNE
> +ifndef VTUNE_AMPLIFIER_XE_2013_DIR
> +$(error VTUNE_AMPLIFIER_XE_2013_DIR undefined)

Is there any way to not duplicate this blob in js/src/shell/Makefile.in?

::: js/src/ion/AsmJS.cpp
@@ +23,5 @@
>  
>  using namespace js::ion;
>  
> +#ifdef MOZ_VTUNE
> +#include "jitprofiling.h"

# include

::: js/src/ion/AsmJSModule.h
@@ +331,5 @@
>  #if defined(JS_CPU_ARM)
>      BoundsCheckVector                     boundsChecks_;
>  #endif
> +#if defined(MOZ_VTUNE)
> +    ProfiledFunctionVector                 profiledFunctions_;

Align 'p' in 'profiledFunctions_'
Attachment #746616 - Flags: review?(luke) → review+
https://hg.mozilla.org/mozilla-central/rev/ce96360f44f5
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.