-
-
Notifications
You must be signed in to change notification settings - Fork 31.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make it possible to use sys.monitoring
for pdb/bdb
#120144
Comments
The general idea seems sound to me. Given the way the code is structured, a per-instance setting seems like it will be the easiest approach at the # in pdb.py
class _PdbBase(bdb.Bdb, cmd.Cmd):
_bdb_use_monitoring = False # Passed to `bdb.Bdb` in `__init__`
...
class PdbTrace(_PdbBase):
...
class PdbMonitoring(_PdbBase):
_bdb_use_monitoring = True # Changes how `Bdb` base class is initialised
...
Pdb = PdbTrace # Use classic sys.settrace-based impl by default
def set_default_pdb(cls):
"""Updated the type referenced by `pdb.Pdb`
Affects the behaviour of the `breakpoint()` builtin, and all
`pdb` module level functions that create `Pdb` instances.
"""
global Pdb
Pdb = cls |
That's surely one way to do it. We can always add a switch to globally flip the backend. I think the only run-time difference is how the code below works: from pdb import Pdb
set_default_pdb('monitoring')
p = Pdb() Is there any other advantage for having two classes? The code structure would be clearer if we have different implementations for I'm just throwing out ideas and maybe it turned out that having two classes is much cleaner. Just wanted to know what's your concern here and why you proposed this. |
Also it would be really helpful if @Yhg1s could provide some feedback. I know you have some concerns about changing |
Feature or enhancement
Proposal:
I had a few proposals to utilize
sys.monitoring
for pdb and all of them were raising concerns. We had a discussion during language summit and I got feedbacks from several members that breaking backwards compatibility for bdb is not acceptible. I can totally understand the concern so I have a much more conservative proposal to do this. I would like to get opinions from people before start working on it.TL; DR:
sys.monitoring
will be an opt-in feature for bdb.Details:
What I propose, is to keep all the code for bdb as it is, and add extra code for
sys.monitoring
without disturbing the existing methods. Everything will be hidden behind a feature gate so all the existing code will work exactly as before.In order to use
sys.monitoring
instead ofsys.settrace
, the user needs to initbdb.Bdb
with an argument (use_monitoring=True
?). Then the underlyingbdb.Bdb
will work insys.monitoring
mode. Ideally, that's the only change the debugger needs to make.Of course, in reality, if the debugger wants to onboard this feature, it may need a few tweaks. For example, in pdb,
debug
command togglessys.settrace
to make it possible to debug something while tracing, that needs to be modified. However, the goal is to make it trivial for debugger developers to switch fromsys.settrace
tosys.monitoring
, if they knew howsys.monitoring
works.Let me re-emphasize it in case that's still a confusion - there's nothing the developer needs to do, if they just want the old
sys.settrace
, everything will simply work because all the new stuff will be behind a feature gate.If they chose to use
sys.monitoring
, there will be a few APIs inbdb.Bdb
that does not even make sense anymore -trace_dispatch
anddispatch_*
functions. The documentation already says:and a normal debugger should not override those methods anyway (pdb does not). Again, those APIs will still work in
sys.settrace
mode.As for pdb, we can also add a feature gate for it. The user can choose between the
sys.settrace
mode and thesys.monitoring
mode. The behaviors in two modes should be identical, except for the performance. We can even make 3.14 a transition version, where the default mechanism is stillsys.settrace
, and the user can opt-in thesys.monitoring
mode by explicitly asking for it (through initialization argument or command line argument). This way, we can get feedbacks from the brave pioneers without disturbing most pdb users. We will have a full year to fix bugs introduced by the mechanism and stablize it.In 3.15, we can make
sys.monitoring
the default and still keepsys.settrace
as a fallback plan.So, why bother? Because we can really gain 100x performance with breakpoints. Not only with breakpoints, even without breakpoints, there's a significant overhead to run with debugger attached:
The overhead with trace attached is 4x-5x for
f()
because thecall
event will still be triggered and even iff_trace == None
, the instrumentation is still there and will be executed! We can have an almost zero-overhead debugger and people are very excited about the possibility.Has this already been discussed elsewhere?
I have already discussed this feature proposal on Discourse
Links to previous discussion of this feature:
#103103
https://discuss.python.org/t/make-pdb-faster-with-pep-669/37674
Linked PRs
sys.monitoring
for bdb and make it default for pdb #124533The text was updated successfully, but these errors were encountered: