Hide variables which fall out-of-scope in SkSL debugger.

We now track each slot's scope depth when it is written, and when scopes
are closed, we remove variables from display which fall out of scope.

Note that we DON'T have a specific trace-op dedicated to setting a
variable's stack depth when it is first declared. This is okay because
the SkVM code generation always stores a zero to a variable's slots as
soon as the variable is declared. This will consistently initialize its
depth to the depth of its initial declaration.

Change-Id: I68f76dfe2930fcd415ba635206cce3d0b94d1aac
Bug: skia:12741
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/484564
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This commit is contained in:
John Stiles 2021-12-15 13:34:44 -05:00 committed by SkCQ
parent 5ef3f98ead
commit b713d81f51
3 changed files with 47 additions and 41 deletions

View File

@ -14,9 +14,9 @@ void SkVMDebugTracePlayer::reset(sk_sp<SkVMDebugTrace> debugTrace) {
fDebugTrace = debugTrace;
fCursor = 0;
fSlots.clear();
fSlots.resize(nslots);
fWriteTime.clear();
fWriteTime.resize(nslots);
fSlots.resize(nslots, {/*fValue=*/0,
/*fScope=*/INT_MAX,
/*fWriteTime=*/0});
fStack.clear();
fStack.push_back({/*fFunction=*/-1,
/*fLine=*/-1,
@ -101,11 +101,11 @@ std::vector<SkVMDebugTracePlayer::VariableData> SkVMDebugTracePlayer::getVariabl
std::vector<VariableData> vars;
bits.forEachSetIndex([&](int slot) {
vars.push_back({slot, fDirtyMask->test(slot), fSlots[slot]});
vars.push_back({slot, fDirtyMask->test(slot), fSlots[slot].fValue});
});
// Order the variable list so that the most recently-written variables are shown at the top.
std::stable_sort(vars.begin(), vars.end(), [&](const VariableData& a, const VariableData& b) {
return fWriteTime[a.fSlotIndex] > fWriteTime[b.fSlotIndex];
return fSlots[a.fSlotIndex].fWriteTime > fSlots[b.fSlotIndex].fWriteTime;
});
return vars;
}
@ -138,7 +138,7 @@ void SkVMDebugTracePlayer::updateVariableWriteTime(int slotIdx, size_t cursor) {
int lastSlotIdx = slotIdx + (changedSlot.columns * changedSlot.rows);
for (; slotIdx < lastSlotIdx; ++slotIdx) {
fWriteTime[slotIdx] = cursor;
fSlots[slotIdx].fWriteTime = cursor;
}
}
@ -159,23 +159,24 @@ bool SkVMDebugTracePlayer::execute(size_t position) {
return true;
}
case SkVMTraceInfo::Op::kVar: { // data: slot, value
int slot = trace.data[0];
int slotIdx = trace.data[0];
int value = trace.data[1];
SkASSERT(slot >= 0);
SkASSERT((size_t)slot < fDebugTrace->fSlotInfo.size());
fSlots[slot] = value;
this->updateVariableWriteTime(slot, position);
if (fDebugTrace->fSlotInfo[slot].fnReturnValue < 0) {
SkASSERT(slotIdx >= 0);
SkASSERT((size_t)slotIdx < fDebugTrace->fSlotInfo.size());
fSlots[slotIdx].fValue = value;
fSlots[slotIdx].fScope = std::min<>(fSlots[slotIdx].fScope, fScope);
this->updateVariableWriteTime(slotIdx, position);
if (fDebugTrace->fSlotInfo[slotIdx].fnReturnValue < 0) {
// Normal variables are associated with the current function.
SkASSERT(fStack.size() > 0);
fStack.rbegin()[0].fDisplayMask.set(slot);
fStack.rbegin()[0].fDisplayMask.set(slotIdx);
} else {
// Return values are associated with the parent function (since the current function
// is exiting and we won't see them there).
SkASSERT(fStack.size() > 1);
fStack.rbegin()[1].fDisplayMask.set(slot);
fStack.rbegin()[1].fDisplayMask.set(slotIdx);
}
fDirtyMask->set(slot);
fDirtyMask->set(slotIdx);
break;
}
case SkVMTraceInfo::Op::kEnter: { // data: function index, (unused)
@ -194,7 +195,17 @@ bool SkVMDebugTracePlayer::execute(size_t position) {
return true;
}
case SkVMTraceInfo::Op::kScope: { // data: scope delta, (unused)
// TODO(skia:12741): track scope depth of variables
SkASSERT(!fStack.empty());
fScope += trace.data[0];
if (trace.data[0] < 0) {
// If the scope is being reduced, discard variables that are now out of scope.
for (size_t slotIdx = 0; slotIdx < fSlots.size(); ++slotIdx) {
if (fScope < fSlots[slotIdx].fScope) {
fSlots[slotIdx].fScope = INT_MAX;
fStack.back().fDisplayMask.reset(slotIdx);
}
}
}
return false;
}
}

View File

@ -75,11 +75,17 @@ private:
int32_t fLine; // our current line number within the function
SkBitSet fDisplayMask; // the variable slots which have been touched in this function
};
struct Slot {
int32_t fValue; // values in each slot
int fScope; // the scope value of each slot
size_t fWriteTime; // when was the variable in this slot most recently written?
// (by cursor position)
};
sk_sp<SkVMDebugTrace> fDebugTrace;
size_t fCursor = 0; // position of the read head
std::vector<int32_t> fSlots; // values in each slot
std::vector<size_t> fWriteTime; // when was the variable in this slot most
// recently written? (cursor position per slot)
int fScope = 0; // the current scope depth (as tracked by
// trace_scope)
std::vector<Slot> fSlots; // the array of all slots
std::vector<StackFrame> fStack; // the execution stack
skstd::optional<SkBitSet> fDirtyMask; // variable slots touched during the most-recently
// executed step

View File

@ -361,17 +361,16 @@ int main() { // Line 2
// We skip over the false-branch.
REPORTER_ASSERT(r, player.getCurrentLine() == 10);
// TODO(skia:12741): `temp` should fall out of scope and disappear
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "##val = 1, temp = 1");
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "##val = 1");
player.step();
// We skip over the true-branch.
REPORTER_ASSERT(r, player.getCurrentLine() == 14);
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val = 1, temp = 1");
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val = 1");
player.step();
REPORTER_ASSERT(r, player.getCurrentLine() == 16);
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "##val = 4, temp = 1");
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "##val = 4");
player.step();
REPORTER_ASSERT(r, player.traceHasCompleted());
@ -419,8 +418,7 @@ int main() { // Line 2
player.step();
REPORTER_ASSERT(r, player.getCurrentLine() == 7);
// TODO(skia:12741): `x` should fall out of scope and disappear
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val = 2, x = 2");
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "val = 2");
player.step();
REPORTER_ASSERT(r, player.traceHasCompleted());
@ -519,40 +517,31 @@ int main() { // Line 2
player.step();
REPORTER_ASSERT(r, player.getCurrentLine() == 9);
// TODO(skia:17421): `c` should fall out of scope
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "##c = 3, b = 2, a = 1");
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "b = 2, a = 1");
player.step();
REPORTER_ASSERT(r, player.getCurrentLine() == 11);
// TODO(skia:17421): `b` and `d` should fall out of scope
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "##d = 4, c = 3, b = 2, a = 1");
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "a = 1");
player.step();
REPORTER_ASSERT(r, player.getCurrentLine() == 13);
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) ==
"##e = 5, d = 4, c = 3, b = 2, a = 1");
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "##e = 5, a = 1");
player.step();
REPORTER_ASSERT(r, player.getCurrentLine() == 15);
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) ==
"##f = 6, e = 5, d = 4, c = 3, b = 2, a = 1");
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "##f = 6, e = 5, a = 1");
player.step();
REPORTER_ASSERT(r, player.getCurrentLine() == 17);
// TODO(skia:17421): `g` should fall out of scope
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) ==
"##g = 7, f = 6, e = 5, d = 4, c = 3, b = 2, a = 1");
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "f = 6, e = 5, a = 1");
player.step();
REPORTER_ASSERT(r, player.getCurrentLine() == 19);
// TODO(skia:17421): `f` and `h` should fall out of scope
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) ==
"##h = 8, g = 7, f = 6, e = 5, d = 4, c = 3, b = 2, a = 1");
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "e = 5, a = 1");
player.step();
REPORTER_ASSERT(r, player.getCurrentLine() == 20);
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) ==
"##i = 9, h = 8, g = 7, f = 6, e = 5, d = 4, c = 3, b = 2, a = 1");
REPORTER_ASSERT(r, make_local_vars_string(*trace, player) == "##i = 9, e = 5, a = 1");
player.step();
REPORTER_ASSERT(r, player.traceHasCompleted());