Skip to content

Commit

Permalink
Don't use ProfPrologues's counters to estimate function invocations f…
Browse files Browse the repository at this point in the history
…or hfsort

Summary:
That can be off because we often skip the prologue when entering JITed code.
Instead, use the counters for the translations that match the function's base
offset.

Reviewed By: mofarrell

Differential Revision: D15935666

fbshipit-source-id: 9b746357b5397871af31a9133273dabd8671b4eb
  • Loading branch information
ottoni authored and hhvm-bot committed Jun 29, 2019
1 parent 65e0d98 commit b654c6c
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 4 deletions.
14 changes: 10 additions & 4 deletions hphp/runtime/vm/jit/mcgen-order.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,21 @@ createCallGraph(jit::hash_map<hfsort::TargetId, FuncId>& funcID) {
FTRACE(3, "createCallGraph: maxFuncId = {}\n", maxFuncId);
for (FuncId fid = 0; fid <= maxFuncId; fid++) {
if (!Func::isFuncIdValid(fid) || !pd->profiling(fid)) continue;
const auto func = Func::fromFuncId(fid);
const auto baseOffset = func->base();
const auto transIds = pd->funcProfTransIDs(fid);
uint32_t size = 1; // avoid zero-sized functions
uint32_t profCount = 0;
for (auto transId : transIds) {
const auto trec = pd->transRec(transId);
assertx(trec->kind() == TransKind::Profile);
size += trec->asmSize();
if (trec->srcKey().offset() == baseOffset) {
profCount += pd->transCounter(transId);
}
}
const auto targetId = cg.addTarget(size);
// NB: avoid division by 0
const auto targetId = cg.addTarget(size, profCount ? profCount : 1);
targetID[fid] = targetId;
funcID[targetId] = fid;
FTRACE(3, " - adding node FuncId = {} => TargetId = {}\n", fid, targetId);
Expand Down Expand Up @@ -105,7 +112,6 @@ createCallGraph(jit::hash_map<hfsort::TargetId, FuncId>& funcID) {
const auto calleeTargetId = targetID[fid];
const auto transIds = pd->funcProfTransIDs(fid);
uint32_t totalCalls = 0;
uint32_t profCount = 1; // avoid zero sample counts
for (int nargs = 0; nargs <= func->numNonVariadicParams() + 1; nargs++) {
auto transId = pd->proflogueTransId(func, nargs);
if (transId == kInvalidTransID) continue;
Expand All @@ -119,9 +125,9 @@ createCallGraph(jit::hash_map<hfsort::TargetId, FuncId>& funcID) {
}
addCallersCount(calleeTargetId, trec->mainCallers(), totalCalls);
addCallersCount(calleeTargetId, trec->guardCallers(), totalCalls);
profCount += pd->transCounter(transId);
}
cg.setSamples(calleeTargetId, std::max(totalCalls, profCount));
auto samples = cg.getSamples(calleeTargetId);
cg.setSamples(calleeTargetId, std::max(totalCalls, samples));
}
cg.normalizeArcWeights();
return cg;
Expand Down
5 changes: 5 additions & 0 deletions hphp/util/hfsort.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ void TargetGraph::setSamples(TargetId id, uint32_t samples) {
targets[id].samples = samples;
}

uint32_t TargetGraph::getSamples(TargetId id) {
assertx(id < targets.size());
return targets[id].samples;
}

const Arc& TargetGraph::incArcWeight(TargetId src, TargetId dst, double w) {
auto res = arcs.emplace(src, dst, w);
if (!res.second) {
Expand Down
1 change: 1 addition & 0 deletions hphp/util/hfsort.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ struct Target {
struct TargetGraph {
TargetId addTarget(uint32_t size, uint32_t samples = 0);
void setSamples(TargetId id, uint32_t samples);
uint32_t getSamples(TargetId id);
const Arc& incArcWeight(TargetId src, TargetId dst, double w = 1.0);
void normalizeArcWeights();

Expand Down

0 comments on commit b654c6c

Please sign in to comment.