-
-
Notifications
You must be signed in to change notification settings - Fork 31.1k
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.monitoringfor 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.monitoringwill 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.monitoringwithout 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.monitoringinstead ofsys.settrace, the user needs to initbdb.Bdbwith an argument (use_monitoring=True?). Then the underlyingbdb.Bdbwill work insys.monitoringmode. 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,
debugcommand togglessys.settraceto 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.settracetosys.monitoring, if they knew howsys.monitoringworks.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.Bdbthat does not even make sense anymore -trace_dispatchanddispatch_*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.settracemode.As for pdb, we can also add a feature gate for it. The user can choose between the
sys.settracemode and thesys.monitoringmode. 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.monitoringmode 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.monitoringthe default and still keepsys.settraceas 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 thecallevent 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.monitoringfor bdb and make it default for pdb #124533The text was updated successfully, but these errors were encountered: