Issue5654
Created on 2009-04-01 16:55 by jpe, last changed 2022-04-11 14:56 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| dicthook.diff | jpe, 2009-04-01 16:55 | Proof of concept | review | |
| testhook.py | jpe, 2009-04-01 16:56 | Test program | ||
| Messages (11) | |||
|---|---|---|---|
| msg85051 - (view) | Author: John Ehresman (jpe) * | Date: 2009-04-01 16:55 | |
I'm interested in adding support for debugger watchpoint's in the core. A watchpoint would cause the debugger to stop when a value in a namespace changes. This hook would be called when PyDict_SetItem & PyDict_DelItem is invoked. I realize that this does not cover function local variables, but I'm more interested in instance attributes and globals. This is a proof of concept patch; I wanted to see if it would work and get some initial feedback. I think the performance implications are minimal in the case where nothing is watched, though the performance of sample callback in _debuggerhooks can be improved. An issue may be if this is used outside a debugger implementation to implement an observer pattern -- I'd like to discourage it, but worry that it will be used since it's there, rather like how the settrace tracer gets used. If there's interest in this, I will clean this up and add watchpoints to pdb. |
|||
| msg85104 - (view) | Author: Martin v. Löwis (loewis) * ![]() |
Date: 2009-04-01 22:41 | |
The patch seems to contain unrelated changes to multiprocessing. |
|||
| msg85120 - (view) | Author: John Ehresman (jpe) * | Date: 2009-04-01 23:25 | |
Oops, the multiprocessing changes should not be in the patch |
|||
| msg85126 - (view) | Author: Raymond Hettinger (rhettinger) * ![]() |
Date: 2009-04-02 01:13 | |
This is a critical path in Python and it should be kept as clean as possible. If something like this goes in, it should be #ifdef'd out by default; otherwise, we have everyone paying the price for a feature that almost no one will use. Strong -1 against this being in the default build. Also, I'm not convinced that the idea itself is even that useful. Why dictionary sets/dels and not list assignments and whatnot. By itself, this one watchpoint hook provides very little utility. We've already cluttered the codebase with various trace and timing options. If something else were to be included, it should be part of a larger, well thought out approach (like what is being done for DTrace). |
|||
| msg85139 - (view) | Author: John Ehresman (jpe) * | Date: 2009-04-02 02:32 | |
My hope is that the runtime performance cost will be negligible but if it isn't, it probably shouldn't go in. The issue with not putting it in another build is that many python debugger users won't want to recompile, so I see it as being of limited use if it's not in the default build. My experience with watchpoints in C debuggers (gdb) is they can be the difference between finding a bug and not -- I recall finding ref counting bugs only because I could watch the ref count and break on the code that modifies it. I would be willing to try and generalize this and support more hooks if there is some interest in it, though watching namespaces is probably the greatest payoff. I think that if will be difficult to remove the hooks currently in the default build because of backward compatibility issues. |
|||
| msg85305 - (view) | Author: Guido van Rossum (gvanrossum) * ![]() |
Date: 2009-04-03 14:07 | |
John, I'm adding a +0 to cancel out Raymond's -1. I've read your motivation and, like you, hope/expect that benchmarking will show this has no real impact. But I need data to decide. Once there are benchmark results I'll revise my view. A good place to start looking for benchmarks might be the Performance section of http://code.google.com/p/unladen-swallow/wiki/ProjectPlan . |
|||
| msg148545 - (view) | Author: Raymond Hettinger (rhettinger) * ![]() |
Date: 2011-11-29 07:36 | |
Can this be closed? |
|||
| msg148551 - (view) | Author: Antoine Pitrou (pitrou) * ![]() |
Date: 2011-11-29 10:43 | |
I see no reason to close until benchmark results prove it decreases real-life performance. Looking at the patch, the cost in the normal case is a single pointer comparison with NULL, something very cheap. |
|||
| msg148595 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2011-11-29 19:11 | |
> the cost in the normal case is a single pointer comparison with NULL, > something very cheap. The Linux kernel patchs dynamically the code: disabling a probe writes NOP instruction in memory to avoid the cost of the test. Probes are completly disabled by default. See kernel markers (introduced in Linux 2.6.24) and the more recent kernel tracepoints (introduced in Linux 2.6.28). http://lwn.net/Articles/300992/ > If something like this goes in, it should be #ifdef'd out > by default I agree. |
|||
| msg148617 - (view) | Author: Martin v. Löwis (loewis) * ![]() |
Date: 2011-11-29 21:40 | |
I think it's besides the point of this patch to ifdef it out. If John wants a version of Python with this enabled, he can well enough build one without our permission. So it would be useful only if it was always available, and perhaps properly integrated into pdb. IOW, I'm +1 for the patch. Having actual benchmarks would demonstrating the (expected) outcome (i.e. no measurable effect) would still be required to advance this. |
|||
| msg365905 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2020-04-07 13:28 | |
No activity for 9 years, I close the issue. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:56:47 | admin | set | github: 49904 |
| 2020-04-07 13:28:52 | vstinner | set | status: open -> closed resolution: rejected messages: + msg365905 stage: needs patch -> resolved |
| 2020-04-07 12:02:16 | piotr.dobrogost | set | nosy:
+ piotr.dobrogost |
| 2011-11-29 21:40:35 | loewis | set | messages: + msg148617 |
| 2011-11-29 19:11:31 | vstinner | set | nosy:
+ vstinner messages: + msg148595 |
| 2011-11-29 10:43:11 | pitrou | set | nosy:
+ pitrou messages: + msg148551 |
| 2011-11-29 07:36:26 | rhettinger | set | messages: + msg148545 |
| 2011-11-29 06:35:46 | ezio.melotti | set | stage: needs patch type: enhancement versions: + Python 3.3, - Python 2.7 |
| 2009-04-03 14:07:15 | gvanrossum | set | nosy:
+ gvanrossum messages: + msg85305 |
| 2009-04-02 02:32:42 | jpe | set | messages: + msg85139 |
| 2009-04-02 01:13:03 | rhettinger | set | nosy:
+ rhettinger messages: + msg85126 |
| 2009-04-01 23:25:54 | jpe | set | nosy:
+ sdeibel messages: + msg85120 |
| 2009-04-01 22:41:49 | loewis | set | nosy:
+ loewis messages: + msg85104 |
| 2009-04-01 16:56:48 | jpe | set | files: + testhook.py |
| 2009-04-01 16:55:04 | jpe | create | |
