Skip to content

Commit

Permalink
- removed stats support to simplify upcoming changes, and given
Browse files Browse the repository at this point in the history
  that they don't really seem useful (anymore);
  • Loading branch information
jaltmayerpizzorno committed Aug 21, 2023
1 parent 8bff925 commit 150813b
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 164 deletions.
25 changes: 2 additions & 23 deletions src/probe.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ class Probe {
bool _signalled;
bool _removed;
int _d_miss_count;
int _u_miss_count;
int _no_signal_count;
int _d_miss_threshold;
std::byte* _code;

Expand All @@ -27,7 +25,7 @@ class Probe {
_sci(PyPtr<>::borrowed(sci)), _filename(PyPtr<>::borrowed(filename)),
_lineno_or_branch(PyPtr<>::borrowed(lineno_or_branch)),
_signalled(false), _removed(false),
_d_miss_count(-1), _u_miss_count(0), _no_signal_count(0),
_d_miss_count(-1),
_d_miss_threshold(PyLong_AsLong(d_miss_threshold)), _code(nullptr) {}


Expand Down Expand Up @@ -84,19 +82,13 @@ class Probe {
}
}
else {
++_u_miss_count;
// U miss
}

Py_RETURN_NONE;
}


PyObject* no_signal() {
++_no_signal_count;
Py_RETURN_NONE;
}


PyObject* mark_removed() {
_removed = true;
Py_RETURN_NONE;
Expand All @@ -109,15 +101,6 @@ class Probe {
Py_RETURN_FALSE;
}

PyObject* get_stats() {
PyPtr<> d_miss_count = PyLong_FromLong(std::max(_d_miss_count, 0));
PyPtr<> u_miss_count = PyLong_FromLong(_u_miss_count);
PyPtr<> total_count = PyLong_FromLong(1 + _d_miss_count + _u_miss_count + _no_signal_count);
return PyTuple_Pack(5, (PyObject*)_filename, (PyObject*)_lineno_or_branch,
(PyObject*)d_miss_count, (PyObject*)u_miss_count,
(PyObject*)total_count);
}

PyObject* set_immediate(PyObject* code_bytes, PyObject* offset) {
#ifdef PYPY_VERSION
PyErr_SetString(PyExc_Exception, "Error: set_immediate does not work with PyPy");
Expand Down Expand Up @@ -168,20 +151,16 @@ probe_set_immediate(PyObject* self, PyObject* const* args, Py_ssize_t nargs) {
}

METHOD_WRAPPER(signal);
METHOD_WRAPPER(no_signal);
METHOD_WRAPPER(mark_removed);
METHOD_WRAPPER(was_removed);
METHOD_WRAPPER(get_stats);


static PyMethodDef methods[] = {
{"new", (PyCFunction)probe_new, METH_FASTCALL, "creates a new probe"},
{"set_immediate", (PyCFunction)probe_set_immediate, METH_FASTCALL, "sets up for immediate removal"},
{"signal", (PyCFunction)probe_signal, METH_FASTCALL, "signals this probe's line or branch was reached"},
{"no_signal", (PyCFunction)probe_no_signal, METH_FASTCALL, "like signal, but called only after this probe is removed"},
{"mark_removed", (PyCFunction)probe_mark_removed, METH_FASTCALL, "marks a probe removed (de-instrumented)"},
{"was_removed", (PyCFunction)probe_was_removed, METH_FASTCALL, "returns whether probe was removed"},
{"get_stats", (PyCFunction)probe_get_stats, METH_FASTCALL, "returns probe stats"},
{NULL, NULL, 0, NULL}
};

Expand Down
3 changes: 1 addition & 2 deletions src/slipcover/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
# intended for slipcover development only
ap.add_argument('--silent', action='store_true', help=argparse.SUPPRESS)
ap.add_argument('--dis', action='store_true', help=argparse.SUPPRESS)
ap.add_argument('--stats', action='store_true', help=argparse.SUPPRESS)
ap.add_argument('--debug', action='store_true', help=argparse.SUPPRESS)
ap.add_argument('--dont-wrap-pytest', action='store_true', help=argparse.SUPPRESS)

Expand Down Expand Up @@ -68,7 +67,7 @@
file_matcher.addOmit(o)


sci = sc.Slipcover(collect_stats=args.stats, immediate=args.immediate,
sci = sc.Slipcover(immediate=args.immediate,
d_miss_threshold=args.threshold, branch=args.branch,
skip_covered=args.skip_covered, disassemble=args.dis)

Expand Down
29 changes: 0 additions & 29 deletions src/slipcover/bytecode.py
Original file line number Diff line number Diff line change
Expand Up @@ -590,35 +590,6 @@ def disable_inserted_function(self, offset):
self.patch[offset] = op_JUMP_FORWARD


def replace_inserted_function(self, offset, new_func_index):
"""Replaces an inserted function by another function."""

assert not self.finished

if self.patch is None:
self.patch = bytearray(self.orig_code.co_code)

assert self.patch[offset] == op_NOP

it = iter(unpack_opargs(self.patch[offset:]))
next(it) # NOP
op_offset, op_len, op, op_arg = next(it)
if op == op_PUSH_NULL:
op_offset, op_len, op, op_arg = next(it)

assert op == op_LOAD_CONST

# FIXME to create a same-length replacement, we need to use the same number
# of EXTENDED_ARG opcodes used to encode op_arg; but arg_ext_needed gives us
# how many are needed at a minimum, not how many were actually used.
# Usually these are the same, but it's possible that unnecessary ones were used.
replacement = opcode_arg(op, new_func_index, arg_ext_needed(op_arg))
assert len(replacement) == op_len

op_offset += offset # we unpacked from co.co_code[offset:]
self.patch[op_offset:op_offset+op_len] = replacement


def replace_global_with_const(self, global_name, const_index):
"""Replaces a global name lookup by a constant load."""
assert not self.finished
Expand Down
59 changes: 2 additions & 57 deletions src/slipcover/slipcover.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,9 @@ def simplify(self, path : str) -> str:


class Slipcover:
def __init__(self, collect_stats: bool = False, immediate: bool = False,
def __init__(self, immediate: bool = False,
d_miss_threshold: int = 50, branch: bool = False, skip_covered: bool = False,
disassemble: bool = False):
self.collect_stats = collect_stats
self.immediate = immediate
self.d_miss_threshold = d_miss_threshold
self.branch = branch
Expand Down Expand Up @@ -65,7 +64,6 @@ def __init__(self, collect_stats: bool = False, immediate: bool = False,
self._get_newly_seen()

self.modules = []
self.all_probes = []

def _get_newly_seen(self):
"""Returns the current set of ``new'' lines, leaving a new container in place."""
Expand Down Expand Up @@ -102,7 +100,6 @@ def instrument(self, co: types.CodeType, parent: types.CodeType = 0) -> types.Co
if isinstance(c, types.CodeType):
ed.set_const(i, self.instrument(c, co))

ed.add_const(probe.no_signal) # used during de-instrumentation
probe_signal_index = ed.add_const(probe.signal)

off_list = list(dis.findlinestarts(co))
Expand Down Expand Up @@ -156,9 +153,6 @@ def instrument(self, co: types.CodeType, parent: types.CodeType = 0) -> types.Co
if self.disassemble:
dis.dis(new_code)

if self.collect_stats:
self.all_probes.extend(probes)

if self.immediate:
for tr, off in zip(probes, ed.get_inserts()):
probe.set_immediate(tr, new_code.co_code, off)
Expand Down Expand Up @@ -210,15 +204,7 @@ def deinstrument(self, co, lines: set) -> types.CodeType:
func_index, func_arg_index, *_ = func
if co_consts[func_index] == probe.signal:
probe.mark_removed(co_consts[func_arg_index])

if self.collect_stats:
# If collecting stats, rather than disabling the probe, we switch to
# calling the 'probe.no_signal' function on it (which we conveniently added
# to the consts before probe.signal, during instrumentation), so that
# we have the total execution count needed for the reports.
ed.replace_inserted_function(offset, func_index-1)
else:
ed.disable_inserted_function(offset)
ed.disable_inserted_function(offset)

new_code = ed.finish()
if new_code is co:
Expand Down Expand Up @@ -249,16 +235,6 @@ def get_coverage(self):

simp = PathSimplifier()

if self.collect_stats:
d_misses = defaultdict(Counter)
u_misses = defaultdict(Counter)
totals = defaultdict(Counter)
for p in self.all_probes:
filename, lineno, d_miss_count, u_miss_count, total_count = probe.get_stats(p)
if d_miss_count: d_misses[filename].update({lineno: d_miss_count})
if u_miss_count: u_misses[filename].update({lineno: u_miss_count})
totals[filename].update({lineno: total_count})

files = dict()
for f, f_code_lines in self.code_lines.items():
if f in self.all_seen:
Expand Down Expand Up @@ -292,21 +268,6 @@ def get_coverage(self):
# the check for den == 0 is just defensive programming... there's always at least 1 line
summary['percent_covered'] = 100.0 if den == 0 else 100*nom/den

if self.collect_stats:
# Once a line reports in, it's available for deinstrumentation.
# Each time it reports in after that, we consider it a miss (like a cache miss).
# We differentiate between (de-instrument) "D misses", where a line
# reports in after it _could_ have been de-instrumented and (use) "U misses"
# and where a line reports in after it _has_ been de-instrumented, but
# didn't use the code object where it's deinstrumented.
f_files['stats'] = {
'd_misses_pct': round(d_misses[f].total()/totals[f].total()*100, 1),
'u_misses_pct': round(u_misses[f].total()/totals[f].total()*100, 1),
'top_d_misses': [f"{it[0]}:{it[1]}" for it in d_misses[f].most_common(5)],
'top_u_misses': [f"{it[0]}:{it[1]}" for it in u_misses[f].most_common(5)],
'top_lines': [f"{it[0]}:{it[1]}" for it in totals[f].most_common(5)],
}

files[simp.simplify(f)] = f_files

summary = {
Expand Down Expand Up @@ -407,22 +368,6 @@ def table(files):
maxcolwidths = [None] * (len(headers)-1) + [missing_width]
print(tabulate(table(cov['files']), headers=headers, maxcolwidths=maxcolwidths), file=outfile)

def stats_table(files):
for f, f_info in sorted(files.items()):
stats = f_info['stats']

yield (f, stats['d_misses_pct'], stats['u_misses_pct'],
" ".join(stats['top_d_misses'][:4]),
" ".join(stats['top_u_misses'][:4]),
" ".join(stats['top_lines'][:4])
)

if self.collect_stats:
print("\n", file=outfile)
print(tabulate(stats_table(cov['files']),
headers=["File", "D miss%", "U miss%", "Top D", "Top U", "Top lines"]),
file=outfile)


@staticmethod
def find_functions(items, visited : set):
Expand Down
Loading

0 comments on commit 150813b

Please sign in to comment.