Skip to content

Commit

Permalink
Output the script ID instead of the script name
Browse files Browse the repository at this point in the history
Summary:
Before, in a heap timeline, instead of a script ID, the script name
was emitted twice. This means if the debugger is currently attached,
the profiler won't link up to the sources being debugged.

The script ID is easy to get, so use it here.

Reviewed By: neildhar

Differential Revision: D23983086

fbshipit-source-id: 502049540e6d993641245edd08391723798efb47
  • Loading branch information
dulinriley authored and facebook-github-bot committed Sep 30, 2020
1 parent 18e5786 commit ea2b5aa
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 19 deletions.
18 changes: 12 additions & 6 deletions include/hermes/VM/StackTracesTree-NoRuntime.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#define HERMES_ENABLE_ALLOCATION_LOCATION_TRACES
#endif

#include "hermes/Public/DebuggerTypes.h"
#include "hermes/Support/OptValue.h"
#include "hermes/Support/StringSetVector.h"

Expand All @@ -43,31 +44,36 @@ struct StackTracesTreeNode {
/// nodes that would appear at the same "location".
struct SourceLoc {
StringSetVector::size_type scriptName;
::facebook::hermes::debugger::ScriptID scriptID;
int32_t lineNo;
int32_t columnNo;
SourceLoc(
StringSetVector::size_type scriptName,
::facebook::hermes::debugger::ScriptID scriptID,
int32_t lineNo,
int32_t columnNo)
: scriptName(scriptName), lineNo(lineNo), columnNo(columnNo){};
: scriptName(scriptName),
scriptID(scriptID),
lineNo(lineNo),
columnNo(columnNo) {}

unsigned hash() const {
return scriptName ^ columnNo ^ lineNo;
return scriptName ^ scriptID ^ columnNo ^ lineNo;
};

bool operator==(const SourceLoc &r) const {
return scriptName == r.scriptName && lineNo == r.lineNo &&
columnNo == r.columnNo;
return scriptName == r.scriptName && scriptID == r.scriptID &&
lineNo == r.lineNo && columnNo == r.columnNo;
}
};

/// Utility class for use with \c llvh::DenseMap .
struct SourceLocMapInfo {
static inline SourceLoc getEmptyKey() {
return {SIZE_MAX, -1, -1};
return {SIZE_MAX, 0, -1, -1};
}
static inline SourceLoc getTombstoneKey() {
return {SIZE_MAX - 1, -1, -1};
return {SIZE_MAX - 1, 0, -1, -1};
}
static unsigned getHashValue(const SourceLoc &v) {
return v.hash();
Expand Down
2 changes: 1 addition & 1 deletion include/hermes/VM/StackTracesTree.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ struct StackTracesTree {
std::unique_ptr<StackTracesTreeNode> root_{new StackTracesTreeNode(
nextNodeID_++,
nullptr, /* parent */
{invalidScriptNameID_, -1, -1},
{invalidScriptNameID_, 0, -1, -1},
nullptr, /* codeBlock */
nullptr, /* ip */
invalidFunctionID_)};
Expand Down
7 changes: 4 additions & 3 deletions lib/VM/HeapSnapshot.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -344,10 +344,10 @@ void HeapSnapshot::emitAllocationTraceInfo() {

struct FuncHashMapInfo {
static StackTracesTreeNode::SourceLoc getEmptyKey() {
return {SIZE_MAX, -1, -1};
return {SIZE_MAX, 0, -1, -1};
}
static inline StackTracesTreeNode::SourceLoc getTombstoneKey() {
return {SIZE_MAX - 1, -1, -1};
return {SIZE_MAX - 1, 0, -1, -1};
}
static unsigned getHashValue(const StackTracesTreeNode::SourceLoc &v) {
return v.hash();
Expand Down Expand Up @@ -383,7 +383,8 @@ void HeapSnapshot::emitAllocationTraceInfo() {
json_.emitValue(functionId); // "function_id"
json_.emitValue(curNode->name); // "name"
json_.emitValue(curNode->sourceLoc.scriptName); // "script_name"
json_.emitValue(curNode->sourceLoc.scriptName); // "script_id"
json_.emitValue(curNode->sourceLoc.scriptID); // "script_id"
// These should be emitted as 1-based, not 0-based like locations.
json_.emitValue(curNode->sourceLoc.lineNo); // "line"
json_.emitValue(curNode->sourceLoc.columnNo); // "column"
for (auto child : curNode->getChildren()) {
Expand Down
3 changes: 2 additions & 1 deletion lib/VM/StackTracesTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ StackTracesTreeNode::SourceLoc StackTracesTree::computeSourceLoc(
// report unknown.
RuntimeModule *runtimeModule = codeBlock->getRuntimeModule();
std::string scriptName;
auto scriptID = runtimeModule->getScriptID();
int32_t lineNo, columnNo;
if (location) {
scriptName = runtimeModule->getBytecode()->getDebugInfo()->getFilenameByID(
Expand All @@ -183,7 +184,7 @@ StackTracesTreeNode::SourceLoc StackTracesTree::computeSourceLoc(
lineNo = -1;
columnNo = -1;
}
return {strings_->insert(scriptName), lineNo, columnNo};
return {strings_->insert(scriptName), scriptID, lineNo, columnNo};
}

void StackTracesTree::pushCallStack(
Expand Down
16 changes: 8 additions & 8 deletions unittests/VMRuntime/HeapSnapshotTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -944,10 +944,10 @@ baz();
EXPECT_STREQ(
fooStackStr.c_str(),
R"#(
global(1) @ test.js(4):2:1
global(2) @ test.js(4):11:4
baz(7) @ test.js(4):9:19
foo(8) @ test.js(4):3:20)#");
global(1) @ test.js(1):2:1
global(2) @ test.js(1):11:4
baz(7) @ test.js(1):9:19
foo(8) @ test.js(1):3:20)#");

auto barAllocNode = FIND_NODE_FOR_ID(barObjID, nodes, strings);
auto barStackTreeNode = idNodeMap.find(barAllocNode.traceNodeID);
Expand All @@ -957,10 +957,10 @@ foo(8) @ test.js(4):3:20)#");
ASSERT_STREQ(
barStackStr.c_str(),
R"#(
global(1) @ test.js(4):2:1
global(2) @ test.js(4):11:4
baz(3) @ test.js(4):9:31
bar(4) @ test.js(4):6:20)#");
global(1) @ test.js(1):2:1
global(2) @ test.js(1):11:4
baz(3) @ test.js(1):9:31
bar(4) @ test.js(1):6:20)#");
}
#endif // HERMES_ENABLE_DEBUGGER

Expand Down

0 comments on commit ea2b5aa

Please sign in to comment.