Reintegrate external profiling support #27466

fisiognomico · 2018-06-06T20:12:07Z

Julia external profiling plus perf support

As presented in #26999, during ORC JIT transition Julia lost the LLVM native support to register JIT events. This was my first activity.

Also with my mentors of the Gsoc we agreed on adding perf support, similarly to what @maleadt did in
#14727 .

Then I have added IntelJITEventListener and OProfile support. OProfile is currently missing line level code annotations, as the current OProfile LLVM support is not implementing it.

That's why I have linked directly the OPprofileJITEventListener code, to avoid having to re-compile LLVM each time 😅 .

This is an example of visualizing the Perf output of runtime code using gprof2dot.

TODO

Work on making an LLVM Patch to add DWARF debug symbols support in Operf.
After that, the OProfileJITEventListener source can be ditched, and the LLVM event listener can be registered directly to the Julia execution engine.
Maybe move the PerfJITEventListener class somewhere else, like in debuginfo.cpp, as it is quite awful linked in the Makefile.
@vtjnash suggested to add some features to the @profile macro. This can be useful, as perf support is intended to profile the whole runtime. I can add a feature so that a particular section of Julia code is profiled with perf events. Also it would not require to compile Julia with perf support.
If anybody fthinks that this can be useful, I will work on that too.

EDIT: The perf wrapper is maintained as an external package.

cc @vtjnash @vchuravy

vchuravy

I would still prefer if we could use the OProfileWrapper from LLVM instead of duplicating it here. Are there any patches we need for that?

vchuravy · 2018-06-06T20:22:49Z

deps/checksums/llvm-6.0.0.src.tar.xz/md5

@@ -0,0 +1 @@
+788a11a35fa62eb008019b37187d09d2


Maybe rebase onto master which already should have these files (You should not check them int)

vchuravy · 2018-06-06T20:23:36Z

src/OProfileJITEventListener.cpp

+			// Build a path from the current entry name
+			SmallString<256> CmdLineFName;
+			raw_svector_ostream(CmdLineFName) << "/proc/" << Entry->d_name
+																				<< "/cmdline";


Formatting seems weird here

fisiognomico · 2018-06-06T20:41:05Z

Well if we want to integrate line level annotations for OProfile yup, I need to patch the LLVM integration.

Either way I agree, the OProfile code will be ditched as it is redundant, and I will register the event from LLVM, as is done with IntelJITEventListener.
I have added it as for the moment is more convenient to test than recompiling every time LLVM 😄

vchuravy

Great! PerfJITEventListener.cpp is from LLVM as well right? If so we should also add this as a patch and any changes that you made should be contributed back upstream.

vchuravy · 2018-06-09T23:50:25Z

deps/llvm.mk

@@ -515,6 +515,8 @@ $(eval $(call LLVM_PATCH,llvm-D38765-gvn_5.0)) # Remove for 6.0
 $(eval $(call LLVM_PATCH,llvm-D42262-jumpthreading-not-i1))
 $(eval $(call LLVM_PATCH,llvm-PPC-addrspaces)) # PPC
 $(eval $(call LLVM_PATCH,llvm-PR36292-5.0)) # PPC fixes #26249, remove for 6.0
+else ifeq ($(LLVM_VER_SHORT),6.0)
+$(eval $(call LLVM_PATCH,llvm-OProfile-line-num)) # In #27466, to be upstreamed


You should probably rebase your branch onto current master so that you have the other 6.0 patches as well

fisiognomico · 2018-06-11T15:40:11Z

The PerfJITEventListener code has been moved to the LLVM patches, now the codebase seems much cleaner! I am still testing a bit with this, in particular you can follow the OProfile patch review at D47925.
For the moment a build with LLVM_USE_VER = 6.0.0 is required, i think that it would be necessary to port it to llvm 3.9.1. The only difference that I notice between the two is DWARF Context definition.

vchuravy · 2018-06-13T21:25:44Z

For the moment a build with LLVM_USE_VER = 6.0.0 is required, i think that it would be necessary to port it to llvm 3.9.1. The only difference that I notice between the two is DWARF Context definition.

Julia moved to LLVM 6.0, so no need to worry about 3.9.1

vchuravy · 2018-06-16T06:44:29Z

You probably also want to squash your commits into one introducing the LLVM patches and one or two for the integration work.

fisiognomico · 2018-06-18T13:12:32Z

Yup it should be cleaner now
Also the OProfile patch has now been upstreamed!

As for a wrapper to Perf events in Julia LinuxPerf.jl will be maintained as an external package, so I took that out of th todo list.

vtjnash

Looks good to me (other than some minor formatting changes). Can you rebase to get CI to re-run? We'll also need BinaryBuilder to ingest these patches. But since this is off by default, I don't think that needs happen before merging.

vtjnash · 2018-07-05T14:52:21Z

src/jitlayers.cpp

@@ -681,7 +682,16 @@ Function *JuliaOJIT::FindFunctionNamed(const std::string &Name)

 void JuliaOJIT::RegisterJITEventListener(JITEventListener *L)
 {
-    // TODO
+    if(!L)


space between if and (

vtjnash · 2018-07-05T14:52:29Z

src/jitlayers.cpp

+void JuliaOJIT::NotifyFinalizer(const object::ObjectFile &Obj,
+                                const RuntimeDyld::LoadedObjectInfo &LoadedObjectInfo)
+{
+    for(auto &Listener : EventListeners)


space between for and (

vtjnash · 2018-07-05T14:53:41Z

src/codegen.cpp

@@ -7474,6 +7474,22 @@ extern "C" void *jl_init_llvm(void)
    jl_setup_module(engine_module);
    jl_setup_module(m);
    return (void*)m;
+
+#ifdef JL_USE_INTEL_JITEVENTS
+    if(jl_using_intel_jitevents)


space between if and (

vtjnash · 2018-07-13T14:58:22Z

(CI failures are unrelated – httpbin.com flakiness and macOS FileWatching)

vchuravy

Please squash the commits and do we need to add https://reviews.llvm.org/D47343 and https://reviews.llvm.org/D44890?

vchuravy · 2018-07-13T17:58:57Z

deps/llvm.mk

@@ -494,6 +498,9 @@ $(eval $(call LLVM_PATCH,llvm-rL332302)) # remove for 7.0
 $(eval $(call LLVM_PATCH,llvm-rL332694)) # remove for 7.0
 $(eval $(call LLVM_PATCH,llvm-rL327898)) # remove for 7.0
 $(eval $(call LLVM_PATCH,llvm-6.0-DISABLE_ABI_CHECKS))
+$(eval $(call LLVM_PATCH,llvm-OProfile-line-num)) # In #27466, to be upstreamed


fisiognomico · 2018-07-13T21:40:04Z

D44890 and D47343 are superfluous as we do object notification in JuliaOJIT!

Moved Oprofile Wrapper, implemented RegisterJITEventListener and NotifyFinalized Added PerfJITEventListener as an LLVM Patch Added IntelJITEventListener Support Added LLVM Patch, removed redundant Oprofile code, wipe blanklines, Register OProfile JITEvent, fixed oprofile compile flag, rebased llvm 6.0 checksums Added Perf profiling support as an LLVM patch.

vchuravy

LGTM, after CI passes

vchuravy reviewed Jun 6, 2018

View reviewed changes

vchuravy reviewed Jun 9, 2018

View reviewed changes

fisiognomico force-pushed the df/operflist branch from 3041de2 to 2cd6ce4 Compare June 11, 2018 15:41

fisiognomico force-pushed the df/operflist branch from d4b88ca to c8f7047 Compare June 18, 2018 11:25

vtjnash reviewed Jul 5, 2018

View reviewed changes

This was referenced Jul 6, 2018

Prepare release for 6.0.1 staticfloat/LLVMBuilder#23

Merged

RFC: Perf JIT event listener #14727

Closed

fisiognomico force-pushed the df/operflist branch from c8f7047 to 7780444 Compare July 12, 2018 11:50

vtjnash approved these changes Jul 13, 2018

View reviewed changes

vtjnash changed the title ~~[WIP] Reintegrate external profiling~~ Reintegrate external profiling support Jul 13, 2018

vchuravy requested changes Jul 13, 2018

View reviewed changes

fisiognomico force-pushed the df/operflist branch from 25821d8 to 781e001 Compare July 13, 2018 21:37

vchuravy force-pushed the df/operflist branch from 781e001 to 8d0fcdb Compare July 19, 2018 04:31

vchuravy approved these changes Jul 19, 2018

View reviewed changes

vchuravy merged commit b012ab3 into JuliaLang:master Jul 19, 2018

fisiognomico mentioned this pull request Jul 25, 2018

Fixes an undef compilation warning #28273

Merged

maleadt mentioned this pull request Jul 25, 2018

External profiling support #26999

Closed

vchuravy mentioned this pull request Jul 26, 2018

Ptrtoint and perf patches staticfloat/LLVMBuilder#25

Merged

fisiognomico mentioned this pull request Aug 2, 2018

Hooked GDB to JITEventListener, cleaned jitlayers GDB interface #28407

Merged

fisiognomico deleted the df/operflist branch August 5, 2018 10:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reintegrate external profiling support #27466

Reintegrate external profiling support #27466

fisiognomico commented Jun 6, 2018 •

edited

Loading

vchuravy left a comment

vchuravy Jun 6, 2018

vchuravy Jun 6, 2018

fisiognomico commented Jun 6, 2018 •

edited

Loading

vchuravy left a comment

vchuravy Jun 9, 2018

fisiognomico commented Jun 11, 2018 •

edited

Loading

vchuravy commented Jun 13, 2018

vchuravy commented Jun 16, 2018

fisiognomico commented Jun 18, 2018

vtjnash left a comment •

edited

Loading

vtjnash Jul 5, 2018

vtjnash Jul 5, 2018

vtjnash Jul 5, 2018

vtjnash commented Jul 13, 2018

vchuravy left a comment

vchuravy Jul 13, 2018

fisiognomico commented Jul 13, 2018

vchuravy left a comment

		@@ -0,0 +1 @@
		788a11a35fa62eb008019b37187d09d2

Reintegrate external profiling support #27466

Reintegrate external profiling support #27466

Conversation

fisiognomico commented Jun 6, 2018 • edited Loading

Julia external profiling plus perf support

TODO

vchuravy left a comment

Choose a reason for hiding this comment

vchuravy Jun 6, 2018

Choose a reason for hiding this comment

vchuravy Jun 6, 2018

Choose a reason for hiding this comment

fisiognomico commented Jun 6, 2018 • edited Loading

vchuravy left a comment

Choose a reason for hiding this comment

vchuravy Jun 9, 2018

Choose a reason for hiding this comment

fisiognomico commented Jun 11, 2018 • edited Loading

vchuravy commented Jun 13, 2018

vchuravy commented Jun 16, 2018

fisiognomico commented Jun 18, 2018

vtjnash left a comment • edited Loading

Choose a reason for hiding this comment

vtjnash Jul 5, 2018

Choose a reason for hiding this comment

vtjnash Jul 5, 2018

Choose a reason for hiding this comment

vtjnash Jul 5, 2018

Choose a reason for hiding this comment

vtjnash commented Jul 13, 2018

vchuravy left a comment

Choose a reason for hiding this comment

vchuravy Jul 13, 2018

Choose a reason for hiding this comment

fisiognomico commented Jul 13, 2018

vchuravy left a comment

Choose a reason for hiding this comment

fisiognomico commented Jun 6, 2018 •

edited

Loading

fisiognomico commented Jun 6, 2018 •

edited

Loading

fisiognomico commented Jun 11, 2018 •

edited

Loading

vtjnash left a comment •

edited

Loading