968510 - Add support for demangling C symbol names in the Firefox profiler?

Status: RESOLVED FIXED

(DevTools :: Performance Tools (Profiler/Timeline), enhancement, P3)

Description

[Jukka Jylänki]

Consider the following Emscripten-compiled application run in the profiler:

http://clb.demon.fi/dump/c_callstack.png

It would be useful to have a checkbox that, when enabled, would show the demangled names of the C symbols in the tree. This would allow easy toggling of callstack versions and locating the appropriate places in code.

You can visit the page that outputs the above profile at 

https://dl.dropboxusercontent.com/u/40949268/emcc/10kCubes_benchmark/10kCubes.html?--benchmark&--objects&1000

The C callstacks in Emscripten follow Clang C++ mangling rules, which are quite standard. Alon Zakai has already implemented in the Emscripten repository at

https://github.com/kripken/emscripten/blob/master/src/preamble.js#L681

a JS version of function demangle(mangledSymbolString) that returns a demangled string of the given function. That could be extracted to a separate file demangle.js for easier co-maintenance between Firefox profiler and the Emscripten repo.

Comment 1

[Jukka Jylänki]

Sorry, mixed up the screenshot and the assigned category between geckoprofiler.xpi and the built-in profiler. Naturally the feature would equally apply to both tools.

Comment 2

[Benoit Girard (:BenWa)]

We wanted to do this but we didn't have a library for doing this. I'll look into it.

Comment 3

[Alon Zakai (:azakai)]

For a library, can use libcxxabi, that's how emscripten does stack traces. If you want, I can provide a JS file that does it. It won't be tiny, though, demanging code is surprisingly large (thousands of lines of LLVM IR, I didn't measure JS). Another option is emscripten's mini-demangler,

https://github.com/kripken/emscripten/blob/master/src/preamble.js#L780

which is handwritten and can demangle "common" stuff, but not everything. Emscripten uses the mini-demangler by default, we include libcxxabi's only when requested, due to code size.

Comment 5

[Jukka Jylänki]

To clarify, this bug report deals with demangling C++ mangling rules for functions, e.g. if I did a JS function with name

function _ZN9MainClass12ProcessFrameEv() {
   // ...;
}

we are asking in this bug that the function name is recognized to be a mangled name under C++ compilation, and it would be shown as function "MainClass::ProcessFrame()" instead. (perhaps with an optional checkbox to toggle demangling on and off)

From the title in bug 1049853, that bug sounds like finding mappings from unknown (JIT-compiled?) stack traces which would otherwise show up as a raw address "0x12345678" back to the actual name of the function?

Comment 8

[Victor Porof [:vporof][:vp]]

(In reply to Jukka Jylänki from comment #5)

You're right. There's bug 1132529 filed for that specifically, and all of these dupes (bug 1049853, bug 1111823) should've been of that one.

Comment 9

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: demangled profiler snapshot

I've got a quick patch using emscripten's quick demangler (not the libcxxxabi one), and certainly has some problems with some function names, but usually they're atleast readable. I would think these are *good enough* to get the right idea, and we can further refine it if needed. They still link to, and show on hover, the mangled name used in JS.

So my questions: is this level of demangling sufficient? Are there other mangled formats we could expect, from Unity or Unreal for example, or another JS-as-a-compilation-target build process?

Comment 10

[Jordan Santell [:jsantell] [@jsantell]]

Also for simplicity, I removed the variant where the passed in mangled name is a number type, as that used some of emscripten's build magic in the file -- is that a variation we could expect? (I've never seen it while profiling emscripten AFAIK)

Comment 11

[Alon Zakai (:azakai)]

Yes, this should be sufficient, mangled names are standard (everywhere relevant that I can think of) and we'd get the same ones from Unity, Unreal, etc. As for the quick mini-demangler, we could improve it if necessary. But optimal might be the compiled complete one, the only downside there is it would be large. Is size an issue?

No need to handle an input number, that's emscripten magic stuff, yeah.

Comment 12

[Jordan Santell [:jsantell] [@jsantell]]

How big is the compiled complete one? Maybe can use the simple one in release/beta, and the compiled one for aurora/nightly due to size. Another possibility maybe is, if it's really large, offer it as an addon, and the tool can check the existence for it and use the addon's demangler (which would be the compiled complete one).

Comment 13

[Alon Zakai (:azakai)]

Looks like 227K for the full one. That's from 5,000 lines of C++.

Comment 14

[Jordan Santell [:jsantell] [@jsantell]]

Can you link the full version? There are other tools that could benefit from this maybe (debugger showing the demangled name in a tooltip or something?) so maybe this could just be included.

Just posted on the mailing list[0] to get thoughts on the filesize vs including it optionally.

[0] https://groups.google.com/forum/#!topic/mozilla.dev.developer-tools/C3aTxnOEaJQ

Comment 15

[Alon Zakai (:azakai)]

Ok, I made a repo for the full version now,

https://github.com/kripken/cxx_demangle

demangle.js there provides demangle().

The size is 263K, after I made a benchmark and noticed that the previous 227K one was quite slow. The current one can do 20,000 demanglings in a second.

Comment 16

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: Screen Shot 2015-12-14 at 9.01.05 AM.png

Looks pretty good! Still waiting to hear back from others regarding large files in m-c.

Some notes:
* In the simple demangler, if string does start with "__Z", we just return the original string name so all function names were passed through the demangler. In the cxx-demangler, if invalid, we get something empty, so we'll need to check before hand (is "__Z" sufficient?), as well as fall back to the original function name if could not demangle (so if there were a non mangled function name that started with __Z or any other function name)
* Need to build with common JS style `module.exports`
* Module.print is not defined -- stubbed this out

The build process I can change, but looks like the C++ source is not included to build, but if the Module.print stuff is removed, I could add the common JS wrappers/any stubs in the before.js/after.js no problem.

Thanks Alon!

Comment 17

[Alon Zakai (:azakai)]

Yes, __Z is sufficient - probably faster anyhow to not call demangle() for anything not starting with that.

There isn't a C++ source in that repo, since the demangling code arrives from an Emscripten system library.

A PR with common JS exports etc. would be great. I really should know more about that stuff...

Screenshot looks great! Makes a huge difference I think for C++ devs to see the demangled names.

Comment 18

[Jukka Jylänki]

Whoa Jordan, this is amazing! Can't wait to have the tool available, looks much clearer.

Can it have a checkbox to toggle the demangling on/off? That would be useful when copypasting text out from the profiler.

I feel that it might be confusing if we used the lightweight handwritten version of stable Firefox, and the compiled demangler for Devedition/Nightly? (double the surface area for bugs, and difficult/silly to document to developer) If size ends up being a concern, perhaps we should just always use the simple one, and start improving it so that it is more complete with demangling all LLVM names?

Comment 19

[Jordan Santell [:jsantell] [@jsantell]]

I think we're going to land it as is. If size is a concern we can deal with that in the future.

Toggling's a good idea! Wrestling with some widget tests now, I think that'll be a follow up that we'll have on by default.

Comment 20

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 968510-demangling.patch

Some notes:
* The flame graph tests rely on ordering of the block data which is based on the frame key I guess? So the order has changed.
* `shouldDemangle` should ideally be on the demangle module, but due to how this is imported, this would be annoying to merge in upstream. Can add to the node module once we have node module loading (Bug 1231453). This is fine for now IMO
* Created bug 1232460 for the follow up on making this a toggleable option

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0b6ec7c6a41f

Comment 21

[Victor Porof [:vporof][:vp]]

Comment on attachment 8698207 [details] [diff] [review]
968510-demangling.patch

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

::: devtools/client/shared/demangle.js
@@ +30,5 @@
> +var ra=[Sc,qc];return{_malloc:vc,_i64Subtract:Cc,_free:wc,_i64Add:Gc,_memmove:Hc,_memset:Bc,___cxa_demangle:Ba,_memcpy:Fc,_bitshift64Lshr:Dc,_bitshift64Shl:Ec,runPostSets:Ac,stackAlloc:sa,stackSave:ta,stackRestore:ua,establishStackSpace:va,setThrew:wa,setTempRet0:za,getTempRet0:Aa,dynCall_iiii:Rc}})
> +
> +
> +// EMSCRIPTEN_END_ASM
> +(Module.asmGlobalArg,Module.asmLibraryArg,buffer);var ___cxa_demangle=Module["___cxa_demangle"]=asm["___cxa_demangle"];var _i64Subtract=Module["_i64Subtract"]=asm["_i64Subtract"];var _free=Module["_free"]=asm["_free"];var runPostSets=Module["runPostSets"]=asm["runPostSets"];var _i64Add=Module["_i64Add"]=asm["_i64Add"];var _memmove=Module["_memmove"]=asm["_memmove"];var _memset=Module["_memset"]=asm["_memset"];var _malloc=Module["_malloc"]=asm["_malloc"];var _memcpy=Module["_memcpy"]=asm["_memcpy"];var _bitshift64Lshr=Module["_bitshift64Lshr"]=asm["_bitshift64Lshr"];var _bitshift64Shl=Module["_bitshift64Shl"]=asm["_bitshift64Shl"];var dynCall_iiii=Module["dynCall_iiii"]=asm["dynCall_iiii"];Runtime.stackAlloc=asm["stackAlloc"];Runtime.stackSave=asm["stackSave"];Runtime.stackRestore=asm["stackRestore"];Runtime.establishStackSpace=asm["establishStackSpace"];Runtime.setTempRet0=asm["setTempRet0"];Runtime.getTempRet0=asm["getTempRet0"];function ExitStatus(status){this.name="ExitStatus";this.message="Program terminated with exit("+status+")";this.status=status}ExitStatus.prototype=new Error;ExitStatus.prototype.constructor=ExitStatus;var initialStackTop;var preloadStartTime=null;var calledMain=false;dependenciesFulfilled=function runCaller(){if(!Module["calledRun"])run();if(!Module["calledRun"])dependenciesFulfilled=runCaller};Module["callMain"]=Module.callMain=function callMain(args){assert(runDependencies==0,"cannot call main when async dependencies remain! (listen on __ATMAIN__)");assert(__ATPRERUN__.length==0,"cannot call main when preRun functions remain to be called");args=args||[];ensureInitRuntime();var argc=args.length+1;function pad(){for(var i=0;i<4-1;i++){argv.push(0)}}var argv=[allocate(intArrayFromString(Module["thisProgram"]),"i8",ALLOC_NORMAL)];pad();for(var i=0;i<argc-1;i=i+1){argv.push(allocate(intArrayFromString(args[i]),"i8",ALLOC_NORMAL));pad()}argv.push(0);argv=allocate(argv,"i32",ALLOC_NORMAL);try{var ret=Module["_main"](argc,argv,0);exit(ret,true)}catch(e){if(e instanceof ExitStatus){return}else if(e=="SimulateInfiniteLoop"){Module["noExitRuntime"]=true;return}else{if(e&&typeof e==="object"&&e.stack)Module.printErr("exception thrown: "+[e,e.stack]);throw e}}finally{calledMain=true}};function run(args){args=args||Module["arguments"];if(preloadStartTime===null)preloadStartTime=Date.now();if(runDependencies>0){return}preRun();if(runDependencies>0)return;if(Module["calledRun"])return;function doRun(){if(Module["calledRun"])return;Module["calledRun"]=true;if(ABORT)return;ensureInitRuntime();preMain();if(Module["onRuntimeInitialized"])Module["onRuntimeInitialized"]();if(Module["_main"]&&shouldRunNow)Module["callMain"](args);postRun()}if(Module["setStatus"]){Module["setStatus"]("Running...");setTimeout((function(){setTimeout((function(){Module["setStatus"]("")}),1);doRun()}),1)}else{doRun()}}Module["run"]=Module.run=run;function exit(status,implicit){if(implicit&&Module["noExitRuntime"]){return}if(Module["noExitRuntime"]){}else{ABORT=true;EXITSTATUS=status;STACKTOP=initialStackTop;exitRuntime();if(Module["onExit"])Module["onExit"](status)}if(ENVIRONMENT_IS_NODE){process["stdout"]["once"]("drain",(function(){process["exit"](status)}));console.log(" ");setTimeout((function(){process["exit"](status)}),500)}else if(ENVIRONMENT_IS_SHELL&&typeof quit==="function"){quit(status)}throw new ExitStatus(status)}Module["exit"]=Module.exit=exit;var abortDecorators=[];function abort(what){if(what!==undefined){Module.print(what);Module.printErr(what);what=JSON.stringify(what)}else{what=""}ABORT=true;EXITSTATUS=1;var extra="\nIf this abort() is unexpected, build with -s ASSERTIONS=1 which can give more information.";var output="abort("+what+") at "+stackTrace()+extra;if(abortDecorators){abortDecorators.forEach((function(decorator){output=decorator(output,what)}))}throw output}Module["abort"]=Module.abort=abort;if(Module["preInit"]){if(typeof Module["preInit"]=="function")Module["preInit"]=[Module["preInit"]];while(Module["preInit"].length>0){Module["preInit"].pop()()}}var shouldRunNow=true;if(Module["noInitialRun"]){shouldRunNow=false}run()

wat

Comment 22

[Carsten Book [:Tomcat]]

Hi, this failed to apply:

applying 968510-demangling.patch
patching file devtools/client/performance/test/browser_profiler_tree-view-02.js
Hunk #2 FAILED at 91
1 out of 3 hunks FAILED -- saving rejects to file devtools/client/performance/test/browser_profiler_tree-view-02.js.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh 968510-demangling.patch

Comment 23

[Jordan Santell [:jsantell] [@jsantell]]

Had to resolve after bug 1081245 -- all fixed and landed

Comment 24

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/6eed749b538e

Comment 25

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6eed749b538e

Comment 26

[Mayur Patil]

[bugday-20160323]

not for beginners. Profiler debugging knowledge required.

Status: RESOLVED,FIXED --> UNVERIFIED

Component: 
Name 	        Firefox
Version 	46.0b2
Build ID 	20160314144540
Update Channel 	beta
User Agent 	Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS              Windows 7 SP1 x86_64