Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C++-demangle JS function names for emscripten'd code #167

Open
mstange opened this issue Feb 27, 2017 · 7 comments
Open

C++-demangle JS function names for emscripten'd code #167

mstange opened this issue Feb 27, 2017 · 7 comments
Labels
feature Work that is user facing, and typically should be planned through https://airtable.com/shrRydo6UXheb help wanted Things ready to be worked on by anyone. Issues must include instructions on how to complete the task

Comments

@mstange
Copy link
Contributor

mstange commented Feb 27, 2017

If you compile a C++ project to JS using emscripten, you end up with JS function names like __ZN20ShaderConstantBuffer9SetFloat3EiRKN4math6float3E. Profiles of such code can be hard to read. We should run function names through a demangler so that they become readable; in the example that would be ShaderConstantBuffer::SetFloat3(int, math::float3 const&).

There's a demangle module on npm that can do this.

┆Issue is synchronized with this Jira Task

@mstange
Copy link
Contributor Author

mstange commented Feb 28, 2017

The old devtools performance tool does this, it was added in bug 968510.

@mstange
Copy link
Contributor Author

mstange commented Feb 28, 2017

Testcase

This is the testcase (from Jukka's dropbox) that was used in that bug.

@gregtatum gregtatum assigned gregtatum and unassigned gregtatum Mar 1, 2017
@gregtatum gregtatum added enhancement help wanted Things ready to be worked on by anyone. Issues must include instructions on how to complete the task labels Mar 1, 2017
@gregtatum gregtatum added feature Work that is user facing, and typically should be planned through https://airtable.com/shrRydo6UXheb and removed feature Work that is user facing, and typically should be planned through https://airtable.com/shrRydo6UXheb enhancement labels Mar 8, 2017
@gregtatum
Copy link
Member

gregtatum commented Jul 20, 2017

I think this would be a good start. We should be able to demangle the names during the profile processing step. I'm not 100% confident how emscripten frames will look during our function processing step.

diff --git a/src/profile-logic/process-profile.js b/src/profile-logic/process-profile.js
index 15c7675..f4d6a62 100644
--- a/src/profile-logic/process-profile.js
+++ b/src/profile-logic/process-profile.js
@@ -8,6 +8,9 @@ import { UniqueStringArray } from '../utils/unique-string-array';
 import { resourceTypes } from './profile-data';
 import { provideHostSide } from '../utils/promise-worker';
 import immutableUpdate from '../utils/immutable-update';
+// Install demangle module
+// https://www.npmjs.com/package/demangle
+import demangleEmscripten from 'demangle';
 import {
   CURRENT_VERSION,
   upgradeProcessedProfileToCurrentVersion,
@@ -247,7 +250,9 @@ function _extractFuncsAndResourcesFromFrames(
           }
 
           if (jsMatch[1]) {
-            funcNameIndex = stringTable.indexForString(jsMatch[1]);
+            funcNameIndex = stringTable.indexForString(
+              _demangleIfNeeded(jsMatch[1])
+            );
           } else {
             // Some JS frames don't have a function because they are for the
             // initial evaluation of the whole JS file. In that case, use the
@@ -571,6 +576,23 @@ function _adjustMarkerTimestamps(
   });
 }
 
+const CHAR_CODE_UNDERSCORE = '_'.charCodeAt(0);
+const CHAR_CODE_CAP_Z = 'Z'.charCodeAt(0);
+
+function _demangleIfNeeded(name: string): string {
+  return _isEmscriptenFunctionName(name) ? demangleEmscripten(name) : name;
+}
+
+function _isEmscriptenFunctionName(name: string): boolean {
+  return (
+    name &&
+    name.charCodeAt &&
+    name.charCodeAt(0) === CHAR_CODE_UNDERSCORE &&
+    name.charCodeAt(1) === CHAR_CODE_UNDERSCORE &&
+    name.charCodeAt(2) === CHAR_CODE_CAP_Z
+  );
+}
+
 /**
  * Convert a profile from the Gecko format into the processed format.
  * Throws an exception if it encounters an incompatible profile.

@mstange what do you think?

@jsantell
Copy link

Let me know if it'd be helpful having access to the npm demangle module/repo!
https://github.com/jsantell/cxx_demangle
https://www.npmjs.com/package/demangle

@gregtatum
Copy link
Member

@jsantell oh yeah, that might be helpful if we need to update anything! Much better to have it as an npm module.

@jsantell
Copy link

@gregtatum added you to owner of npm module -- and I think Alon's repo is the source of truth here (rather than my fork)

@mstange
Copy link
Contributor Author

mstange commented Sep 4, 2021

In the meantime we have added a demangling dependency: https://www.npmjs.com/package/gecko-profiler-demangle

This should be quite simple to implement. I think we'd just need an example profile to test it on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Work that is user facing, and typically should be planned through https://airtable.com/shrRydo6UXheb help wanted Things ready to be worked on by anyone. Issues must include instructions on how to complete the task
Projects
None yet
Development

No branches or pull requests

3 participants