classification
Title: improve tracing performances when f_trace is NULL
Type: enhancement Stage: patch review
Components: Interpreter Core Versions: Python 3.4
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: benjamin.peterson Nosy List: asvetlov, belopolsky, benjamin.peterson, georg.brandl, jcea, pitrou, python-dev, xdegaye
Priority: normal Keywords: patch

Created on 2012-12-12 21:37 by xdegaye, last changed 2013-12-21 14:05 by xdegaye. This issue is now closed.

Files
File name Uploaded Description Edit
f_trace_perfs.diff xdegaye, 2012-12-12 21:37 review
foo.py xdegaye, 2012-12-12 21:38
bar.py xdegaye, 2012-12-12 21:38
f_trace_perfs-2.7.patch xdegaye, 2013-01-22 14:31
revert_f_trace_perfs.patch xdegaye, 2013-01-23 13:26 review
Messages (15)
msg177390 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2012-12-12 21:37
When f_trace is NULL, only PyTrace_CALL tracing events trigger the invocation of
the trace function (see trace_trampoline).
maybe_call_line_trace() does quite some work though _PyCode_CheckLineNumber to
find out if the instruction should be traced, and all this work is useless when f_trace
is NULL. The patch checks for f_trace on all tracing events that are not
PyTrace_CALL.

The performance gain with the following test is about 30%. The first test is
with python on the default branch, the second one with the patch applied.
Note: a breakpoint is set at line 1 to ensure that tracing is enabled, without
any breakpoint, pdb sets the trace function to None after a continue.

$ ./python -m pdb /tmp/foo.py
> /tmp/foo.py(1)<module>()
-> import timeit
(Pdb) break 1
Breakpoint 1 at /tmp/foo.py:1
(Pdb) continue
9.226385930000106
The program finished and will be restarted
> /tmp/foo.py(1)<module>()
-> import timeit
(Pdb) q
[62276 refs]


$ ./python -m pdb /tmp/foo.py 
> /tmp/foo.py(1)<module>()
-> import timeit
(Pdb) break 1
Breakpoint 1 at /tmp/foo.py:1
(Pdb) continue
7.199809867001022
The program finished and will be restarted
> /tmp/foo.py(1)<module>()
-> import timeit
(Pdb)
msg177391 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2012-12-12 21:48
> When f_trace is NULL, only PyTrace_CALL tracing events trigger the invocation
> of the trace function (see trace_trampoline).

This may be confusing. I meant that when f_trace is NULL, PyTrace_LINE,
PyTrace_RETURN and PyTrace_EXCEPTION are not traced.
msg178082 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-12-24 17:54
New changeset 1fb26db7d719 by Benjamin Peterson in branch 'default':
improve tracing performance when f_trace is NULL (closes #16672)
http://hg.python.org/cpython/rev/1fb26db7d719
msg180391 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2013-01-22 14:31
Attached is a patch for the current head of 2.7.
It would nice to have this patch on 2.7 too.

With this patch, an implementation of pdb running on 2.7 with an
extension module, runs at 1.2 times the speed of the interpreter when
the trace function is active (see
http://code.google.com/p/pdb-clone/wiki/Performances). The performance
gain is 30%.
msg180394 - (view) Author: Jesús Cea Avión (jcea) * (Python committer) Date: 2013-01-22 14:36
Benjamin, ans the previous commiter, could you possibly check the 2.7 proposed patch?
msg180395 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2013-01-22 14:46
This patch causes test_hotshot to fail.
msg180401 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-01-22 15:26
I don't think performance patches should be committed to bugfix branches (especially 2.7 which is in slow maintenance mode). Recommend closing.
msg180402 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2013-01-22 15:38
That, too.
msg180408 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2013-01-22 16:06
One may argue that this is not only a performances patch and that it
fixes the wasting of cpu resources when tracing is on. Wasting cpu
resources is a bug. Anyway, this is fine with me to close this minor
issue on 2.7.

The test_hotshot test is ok on my linux box and with the patch applied
on 2.7 head. Just curious to know what the problem is.

And thanks for applying the patch to 3.4.
msg180459 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2013-01-23 13:26
The patch applied to the default branch should be reverted.

The 2.7 _hotshot extension module follows the specifications of PyEval_SetTrace:

  """Set the tracing function to func. This is similar to PyEval_SetProfile(),
  except the tracing function does receive line-number events."""
                              ^^^^^^^^^^^^

The 2.7 patch breaks test_hotshot because PyTrace_LINE events are not sent
anymore when f_trace is NULL.

The first patch (already applied to the default branch), breaks also existing
applications that expect to receive line events when they don't care to
implement the following semantics defined by sys.settrace:

  """The trace function is invoked (with event set to 'call') whenever a new
     local scope is entered; it should return a reference to a local trace
     function to be used that scope, or None if the scope shouldn’t be
     traced."""

Those applications want to receive unconditionally line debug events.

Attached is a patch that reverts the patch on the default branch.
msg180460 - (view) Author: Jesús Cea Avión (jcea) * (Python committer) Date: 2013-01-23 13:32
So, Xavier, are you saying that you are reverting the patch?.

Could be possible to provide a "good" patch, with a correct test of the situation you describe?

Or are you suggesting just revert this and close this bugentry for good?.
msg180461 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-01-23 13:38
New changeset cd87afe18ff8 by Benjamin Peterson in branch 'default':
revert #16672 for incorrect semantics
http://hg.python.org/cpython/rev/cd87afe18ff8
msg180462 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2013-01-23 13:49
It is not possible to improve the performances of the trace function set with
sys.settrace without breaking backward compatibility for PyEval_SetTrace or
without introducing a new PyEval_xxx of some sort.

Yes, I suggest to revert this patch.
msg180463 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2013-01-23 13:58
status should be close, I guess.
msg206736 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2013-12-21 14:05
A patch proposed in issue 20041 provides a backward compatible solution to this performance enhancement.
History
Date User Action Args
2013-12-21 14:05:20xdegayesetmessages: + msg206736
2013-01-23 15:09:44benjamin.petersonsetstatus: open -> closed
resolution: rejected
2013-01-23 13:58:57xdegayesetmessages: + msg180463
2013-01-23 13:49:04xdegayesetmessages: + msg180462
2013-01-23 13:38:55python-devsetmessages: + msg180461
2013-01-23 13:32:22jceasetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg180460
2013-01-23 13:26:58xdegayesetfiles: + revert_f_trace_perfs.patch

messages: + msg180459
2013-01-22 16:06:36xdegayesetmessages: + msg180408
2013-01-22 15:38:41benjamin.petersonsetstatus: open -> closed
resolution: fixed
messages: + msg180402
2013-01-22 15:26:17pitrousetnosy: + pitrou
messages: + msg180401
2013-01-22 14:46:36benjamin.petersonsetmessages: + msg180395
2013-01-22 14:36:41jceasetmessages: + msg180394
2013-01-22 14:36:03jceasetstatus: closed -> open

nosy: + benjamin.peterson
assignee: benjamin.peterson
resolution: fixed -> (no value)
stage: resolved -> patch review
2013-01-22 14:31:13xdegayesetfiles: + f_trace_perfs-2.7.patch

messages: + msg180391
2012-12-30 14:36:02jceasetnosy: + jcea
2012-12-24 17:54:27python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg178082

resolution: fixed
stage: resolved
2012-12-24 16:20:18asvetlovsetnosy: + asvetlov
2012-12-15 15:43:31pitrousetnosy: + georg.brandl, belopolsky
2012-12-12 21:48:51xdegayesetmessages: + msg177391
2012-12-12 21:38:10xdegayesetfiles: + bar.py
2012-12-12 21:38:01xdegayesetfiles: + foo.py
2012-12-12 21:37:38xdegayecreate