Skip to content

Commit 7e46a72

Browse files
committed
Reapply "[Dexter] Improve performance by evaluating expressions only when needed"
Fixes issue found on greendragon buildbot, in which an incorrectly indented statement following an if block led to entire frames being dropped instead of simply filtering unneeded watches. This reverts commit 1f44fa3.
1 parent 98a95d4 commit 7e46a72

File tree

7 files changed

+78
-25
lines changed

7 files changed

+78
-25
lines changed

cross-project-tests/debuginfo-tests/dexter/dex/command/CommandBase.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,11 @@
1010
"""
1111

1212
import abc
13+
from collections import namedtuple
1314
from typing import List
1415

16+
StepExpectInfo = namedtuple('StepExpectInfo', 'expression, path, frame_idx, line_range')
17+
1518
class CommandBase(object, metaclass=abc.ABCMeta):
1619
def __init__(self):
1720
self.path = None

cross-project-tests/debuginfo-tests/dexter/dex/command/commands/DexExpectProgramState.py

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
from itertools import chain
1212

13-
from dex.command.CommandBase import CommandBase
13+
from dex.command.CommandBase import CommandBase, StepExpectInfo
1414
from dex.dextIR import ProgramState, SourceLocation, StackFrame, DextIR
1515

1616
def frame_from_dict(source: dict) -> StackFrame:
@@ -56,9 +56,23 @@ def get_name():
5656
return __class__.__name__
5757

5858
def get_watches(self):
59-
frame_expects = chain.from_iterable(frame.watches
60-
for frame in self.expected_program_state.frames)
61-
return set(frame_expects)
59+
frame_expects = set()
60+
for idx, frame in enumerate(self.expected_program_state.frames):
61+
path = (frame.location.path if
62+
frame.location and frame.location.path else self.path)
63+
line_range = (
64+
range(frame.location.lineno, frame.location.lineno + 1)
65+
if frame.location and frame.location.lineno else None)
66+
for watch in frame.watches:
67+
frame_expects.add(
68+
StepExpectInfo(
69+
expression=watch,
70+
path=path,
71+
frame_idx=idx,
72+
line_range=line_range
73+
)
74+
)
75+
return frame_expects
6276

6377
def eval(self, step_collection: DextIR) -> bool:
6478
for step in step_collection.steps:

cross-project-tests/debuginfo-tests/dexter/dex/command/commands/DexExpectWatchBase.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,13 @@
1212
import abc
1313
import difflib
1414
import os
15+
from collections import namedtuple
1516

16-
from dex.command.CommandBase import CommandBase
17+
from dex.command.CommandBase import CommandBase, StepExpectInfo
1718
from dex.command.StepValueInfo import StepValueInfo
1819

1920

21+
2022
class DexExpectWatchBase(CommandBase):
2123
def __init__(self, *args, **kwargs):
2224
if len(args) < 2:
@@ -68,7 +70,7 @@ def __init__(self, *args, **kwargs):
6870

6971

7072
def get_watches(self):
71-
return [self.expression]
73+
return [StepExpectInfo(self.expression, self.path, 0, range(self._from_line, self._to_line + 1))]
7274

7375
@property
7476
def line_range(self):
@@ -149,11 +151,11 @@ def _check_watch_order(self, actual_watches, expected_values):
149151
return differences
150152

151153
def eval(self, step_collection):
154+
assert os.path.exists(self.path)
152155
for step in step_collection.steps:
153156
loc = step.current_location
154157

155158
if (loc.path and os.path.exists(loc.path) and
156-
os.path.exists(self.path) and
157159
os.path.samefile(loc.path, self.path) and
158160
loc.lineno in self.line_range):
159161
try:

cross-project-tests/debuginfo-tests/dexter/dex/debugger/DebuggerBase.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,26 @@
1313
import unittest
1414

1515
from types import SimpleNamespace
16+
from dex.command.CommandBase import StepExpectInfo
1617
from dex.dextIR import DebuggerIR, FrameIR, LocIR, StepIR, ValueIR
1718
from dex.utils.Exceptions import DebuggerException
1819
from dex.utils.Exceptions import NotYetLoadedDebuggerException
1920
from dex.utils.ReturnCode import ReturnCode
2021

22+
def watch_is_active(watch_info: StepExpectInfo, path, frame_idx, line_no):
23+
_, watch_path, watch_frame_idx, watch_line_range = watch_info
24+
# If this watch should only be active for a specific file...
25+
if watch_path and os.path.isfile(watch_path):
26+
# If the current path does not match the expected file, this watch is
27+
# not active.
28+
if not (path and os.path.isfile(path) and
29+
os.path.samefile(path, watch_path)):
30+
return False
31+
if watch_frame_idx != frame_idx:
32+
return False
33+
if watch_line_range and line_no not in list(watch_line_range):
34+
return False
35+
return True
2136

2237
class DebuggerBase(object, metaclass=abc.ABCMeta):
2338
def __init__(self, context):

cross-project-tests/debuginfo-tests/dexter/dex/debugger/dbgeng/dbgeng.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import os
1010
import platform
1111

12-
from dex.debugger.DebuggerBase import DebuggerBase
12+
from dex.debugger.DebuggerBase import DebuggerBase, watch_is_active
1313
from dex.dextIR import FrameIR, LocIR, StepIR, StopReason, ValueIR
1414
from dex.dextIR import ProgramState, StackFrame, SourceLocation
1515
from dex.utils.Exceptions import DebuggerException, LoadDebuggerException
@@ -133,8 +133,14 @@ def _get_step_info(self, watches, step_index):
133133
column=0),
134134
watches={})
135135
for expr in map(
136-
lambda watch, idx=i: self.evaluate_expression(watch, idx),
137-
watches):
136+
# Filter out watches that are not active in the current frame,
137+
# and then evaluate all the active watches.
138+
lambda watch_info, idx=i:
139+
self.evaluate_expression(watch_info.expression, idx),
140+
filter(
141+
lambda watch_info, idx=i, line_no=loc.lineno, path=loc.path:
142+
watch_is_active(watch_info, path, idx, line_no),
143+
watches)):
138144
state_frame.watches[expr.expression] = expr
139145
state_frames.append(state_frame)
140146

cross-project-tests/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from subprocess import CalledProcessError, check_output, STDOUT
1313
import sys
1414

15-
from dex.debugger.DebuggerBase import DebuggerBase
15+
from dex.debugger.DebuggerBase import DebuggerBase, watch_is_active
1616
from dex.dextIR import FrameIR, LocIR, StepIR, StopReason, ValueIR
1717
from dex.dextIR import StackFrame, SourceLocation, ProgramState
1818
from dex.utils.Exceptions import DebuggerException, LoadDebuggerException
@@ -208,6 +208,7 @@ def _get_step_info(self, watches, step_index):
208208
'column': sb_line.GetColumn()
209209
}
210210
loc = LocIR(**loc_dict)
211+
valid_loc_for_watch = loc.path and os.path.exists(loc.path)
211212

212213
frame = FrameIR(
213214
function=function, is_inlined=sb_frame.IsInlined(), loc=loc)
@@ -223,10 +224,17 @@ def _get_step_info(self, watches, step_index):
223224
is_inlined=frame.is_inlined,
224225
location=SourceLocation(**loc_dict),
225226
watches={})
226-
for expr in map(
227-
lambda watch, idx=i: self.evaluate_expression(watch, idx),
228-
watches):
229-
state_frame.watches[expr.expression] = expr
227+
if valid_loc_for_watch:
228+
for expr in map(
229+
# Filter out watches that are not active in the current frame,
230+
# and then evaluate all the active watches.
231+
lambda watch_info, idx=i:
232+
self.evaluate_expression(watch_info.expression, idx),
233+
filter(
234+
lambda watch_info, idx=i, line_no=loc.lineno, loc_path=loc.path:
235+
watch_is_active(watch_info, loc_path, idx, line_no),
236+
watches)):
237+
state_frame.watches[expr.expression] = expr
230238
state_frames.append(state_frame)
231239

232240
if len(frames) == 1 and frames[0].function is None:

cross-project-tests/debuginfo-tests/dexter/dex/debugger/visualstudio/VisualStudio.py

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@
1111
import os
1212
import sys
1313
from pathlib import PurePath
14-
from collections import namedtuple
15-
from collections import defaultdict
14+
from collections import defaultdict, namedtuple
1615

17-
from dex.debugger.DebuggerBase import DebuggerBase
16+
from dex.command.CommandBase import StepExpectInfo
17+
from dex.debugger.DebuggerBase import DebuggerBase, watch_is_active
1818
from dex.dextIR import FrameIR, LocIR, StepIR, StopReason, ValueIR
1919
from dex.dextIR import StackFrame, SourceLocation, ProgramState
2020
from dex.utils.Exceptions import Error, LoadDebuggerException
@@ -244,6 +244,9 @@ def _get_step_info(self, watches, step_index):
244244
state_frames = []
245245

246246

247+
loc = LocIR(**self._location)
248+
valid_loc_for_watch = loc.path and os.path.exists(loc.path)
249+
247250
for idx, sf in enumerate(stackframes):
248251
frame = FrameIR(
249252
function=self._sanitize_function_name(sf.FunctionName),
@@ -254,20 +257,20 @@ def _get_step_info(self, watches, step_index):
254257
if any(name in fname for name in self.frames_below_main):
255258
break
256259

257-
258260
state_frame = StackFrame(function=frame.function,
259261
is_inlined=frame.is_inlined,
260262
watches={})
261263

262-
for watch in watches:
263-
state_frame.watches[watch] = self.evaluate_expression(
264-
watch, idx)
264+
if valid_loc_for_watch and idx == 0:
265+
for watch_info in watches:
266+
if watch_is_active(watch_info, loc.path, idx, loc.lineno):
267+
watch_expr = watch_info.expression
268+
state_frame.watches[watch_expr] = self.evaluate_expression(watch_expr, idx)
265269

266270

267271
state_frames.append(state_frame)
268272
frames.append(frame)
269273

270-
loc = LocIR(**self._location)
271274
if frames:
272275
frames[0].loc = loc
273276
state_frames[0].location = SourceLocation(**self._location)
@@ -298,9 +301,11 @@ def frames_below_main(self):
298301
]
299302

300303
def evaluate_expression(self, expression, frame_idx=0) -> ValueIR:
301-
self.set_current_stack_frame(frame_idx)
304+
if frame_idx != 0:
305+
self.set_current_stack_frame(frame_idx)
302306
result = self._debugger.GetExpression(expression)
303-
self.set_current_stack_frame(0)
307+
if frame_idx != 0:
308+
self.set_current_stack_frame(0)
304309
value = result.Value
305310

306311
is_optimized_away = any(s in value for s in [

0 commit comments

Comments
 (0)