Author gwk
Recipients gwk, haypo
Date 2017-01-31.18:20:51
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1485886852.16.0.854514177291.issue29400@psf.upfronthosting.co.za>
In-reply-to
Content
I have recently put some effort into developing a code coverage tool that shows correct results for intraline branches. In order to get intraline trace data, I patched CPython, adding an optional "trace instructions" flag to sys.settrace. The patch is fairly simple.

http://github.com/gwk/cpython/tree/inst-tracing

The crucial detail in ceval is here:

@@ -4521,6 +4523,9 @@ maybe_call_line_trace(Py_tracefunc func, PyObject *obj,
         frame->f_lineno = line;
         result = call_trace(func, obj, tstate, frame, PyTrace_LINE, Py_None);
     }
+    else if (tstate->inst_tracing) {
+        result = call_trace(func, obj, tstate, frame, PyTrace_INSTRUCTION, Py_None);
+    }

The call to maybe_call_line_trace is guarded by several checks, so I do not see how this would significantly impact performance, except for when tracing is enabled. I built python3.6 using clang -O3 -flto (I saw lots of test failures during the --with-optimizations PGO tests so I opted for a simpler build), with and without my patch. I then ran `python3 -m performance run --fast` and manually compared the results. The patched version was in most cases slightly slower but within the standard deviation of the normal version.

I would appreciate feedback before going any further with this. I looked at haypo's perf project but was unsure how to use it to run a more detailed comparison.

* First, will this feature be rejected on performance grounds or some other obvious reason?

* Second, does setting `tstate->inst_tracing` in PyEval_SetTraceInstructions require more elaborate guards similar to those in PyEval_SetTrace? I do not understand the intricacies of that function.

I am also curious about backwards compatibility. This adds an optional argument to settrace (haypo's recommendation) and some other things that appear to be under Py_LIMITED_API or internal (but I am not sure).
* Is a change like this eligible for 3.6.x or would it have to wait until 3.7?
* Would it be better to add a completely new function, e.g. sys.settrace_instructions instead of changing the signature of settrace?

Lastly, I have a few ideas for enhancing this patch that will require some more work on my end to verify their utility.
In particular:
* Remove the `else` to always trace with PyTrace_INSTRUCTION, rather than as an alternate to PyTrace_LINE as it does now.
* Pass the previous instruction offset as the trace function argument. This would give the tracer complete "edge" information. (In my coverage tool's trace function I track the previous edge as a nonlocal, but it would be better/faster to do it automatically in maybe_call_line_trace).
* Finally, if all of this works, it might also be useful (as a performance enhancement) to add a PyTrace_BRANCH option, which would omit runs of opcodes inbetween branches, as these edges tend not be be useful, and even counterproductive (at least for my use case). However it might be too tricky to get all the cases right.
History
Date User Action Args
2017-01-31 18:20:52gwksetrecipients: + gwk, haypo
2017-01-31 18:20:52gwksetmessageid: <1485886852.16.0.854514177291.issue29400@psf.upfronthosting.co.za>
2017-01-31 18:20:52gwklinkissue29400 messages
2017-01-31 18:20:51gwkcreate