| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
But does it matter? There is no promise that sys.monitoring is a module. It's just a namespace ("let's do more of those!"). You can write import sys; sys.monitoring.set_events(...) or you can use from sys import modules; modules.set_events(...). I'd say having it be a module is better than having it be a class full of static methods. :-) |
Sorry, something went wrong.
This happens basically whenever a module imports another module: >>> import ast
>>> type(ast.sys)
<class 'module'>
>>> import ast.sys
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ModuleNotFoundError: No module named 'ast.sys'; 'ast' is not a package🙃 Perhaps a good mental model for sys.monitoring is that the top line of the sys module is import _something_super_secret as monitoring. |
Sorry, something went wrong.
|
It is a bit bit weird that from sys import monitoring as m doesn't work. I expect to (hopefully before 3.12) use some sort of lazy loading to avoid creating the monitoring object if it isn't needed. |
Sorry, something went wrong.
|
@fabioz Thanks for the reproducer. |
Sorry, something went wrong.
|
OK. I'm merging this. I think this is stable enough to merge, and we can probably get better bug reports with this merged than on a branch. |
Sorry, something went wrong.
|
@markshannon from the fix you seem to have put there, it'd still fail if the user set the tracing to None and then back to the actual trace function... |
Sorry, something went wrong.
|
I think that is the current behavior, is it not? If you restart tracing, then you want a line event for the current line. TBH, it is all an undocumented black box, so some guess work is required. |
Sorry, something went wrong.
|
Darn, I was just about to complete my second review. $ ./python.exe -m test -R : test_monitoring # <= fails |
Sorry, something went wrong.
| Having stacktop <= 0 ensures that invalid | ||
| values are not visible to the cycle GC. | ||
| We choose -1 rather than 0 to assist debugging. */ |
There was a problem hiding this comment.
| Having stacktop <= 0 ensures that invalid | |
| values are not visible to the cycle GC. | |
| We choose -1 rather than 0 to assist debugging. */ | |
| Having stacktop <= 0 ensures that invalid | |
| values are not visible to the cycle GC. | |
| We choose -1 rather than 0 to assist debugging. */ |
Sorry, something went wrong.
| DTRACE_FUNCTION_ENTRY(); | ||
| /* Because this avoids the RESUME, | ||
| * we need to update instrumentation */ | ||
| _Py_Instrument(frame->f_code, tstate->interp); |
There was a problem hiding this comment.
The return value of _Py_Instrument is ignored here. Since it might set an exception, I'd expect a goto exit_unwind on error here.
Sorry, something went wrong.
There was a problem hiding this comment.
We are already handling an exception here, so the exception isn't lost, it replaces the thrown exception.
The question is whether to raise it back to the caller, or to inject into the coroutine/generator?
If unwinding were done in a separate function from evaluation, then the thrown exception would unwind the stack, then the new exception would be raised on continued execution, which would be better.
I'm inclined to leave it for now, and let it get fixed as a side effect of separating evaluation and unwinding.
Sorry, something went wrong.
| }; | ||
|
|
||
| static inline bool | ||
| opcode_has_event(int opcode) { |
There was a problem hiding this comment.
| opcode_has_event(int opcode) { | |
| opcode_has_event(uint8_t opcode) | |
| { |
Sorry, something went wrong.
| } | ||
|
|
||
| static inline bool | ||
| is_instrumented(int opcode) { |
There was a problem hiding this comment.
| is_instrumented(int opcode) { | |
| is_instrumented(uint8_t opcode) | |
| { |
Sorry, something went wrong.
| assert(test); \ | ||
| } while (0) | ||
|
|
||
| bool valid_opcode(int opcode) { |
There was a problem hiding this comment.
| bool valid_opcode(int opcode) { | |
| bool valid_opcode(int opcode) | |
| { |
Sorry, something went wrong.
| _PyInterpreterFrame *frame, _Py_CODEUNIT *instr, _Py_CODEUNIT *target | ||
| ) { |
There was a problem hiding this comment.
| _PyInterpreterFrame *frame, _Py_CODEUNIT *instr, _Py_CODEUNIT *target | |
| ) { | |
| _PyInterpreterFrame *frame, _Py_CODEUNIT *instr, _Py_CODEUNIT *target) | |
| { |
Sorry, something went wrong.
|
|
||
| #define C_RETURN_EVENTS \ | ||
| ((1 << PY_MONITORING_EVENT_C_RETURN) | \ | ||
| (1 << PY_MONITORING_EVENT_C_RAISE)) |
There was a problem hiding this comment.
| (1 << PY_MONITORING_EVENT_C_RAISE)) | |
| (1 << PY_MONITORING_EVENT_C_RAISE)) |
Sorry, something went wrong.
| interp->monitoring_tool_names[tool_id] == NULL | ||
| ) { |
There was a problem hiding this comment.
| interp->monitoring_tool_names[tool_id] == NULL | |
| ) { | |
| interp->monitoring_tool_names[tool_id] == NULL) | |
| { |
Sorry, something went wrong.
| else { | ||
| return MOST_SIGNIFICANT_BITS[bits]; | ||
| } |
There was a problem hiding this comment.
| else { | |
| return MOST_SIGNIFICANT_BITS[bits]; | |
| } | |
| return MOST_SIGNIFICANT_BITS[bits]; |
Sorry, something went wrong.
|
|
||
| /* Should use instruction metadata for this */ | ||
| static bool | ||
| is_super_instruction(int opcode) { |
There was a problem hiding this comment.
| is_super_instruction(int opcode) { | |
| is_super_instruction(uint8_t opcode) | |
| { |
Sorry, something went wrong.
Ok, I'll try another round of the debugger tests to see if there's more breakage -- that previous issue didn't really let me get further, so, I'll check and report back -- with bugs in the tracker if that's the case I guess ;) |
Sorry, something went wrong.
|
@markshannon I just checked and changing the tracing shouldn't duplicate line events in the new tracer (it should only report when the line actually changes). I created #103471 with a test case for this (which works in Python 3.11 and fails in the current master). |
Sorry, something went wrong.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️Hi! The buildbot AMD64 Arch Linux TraceRefs 3.x has failed when building commit 411b169. What do you need to do:
You can take a look at the buildbot page here: https://buildbot.python.org/all/#builders/484/builds/3091 Summary of the results of the build (if available): Click to see traceback logsNote: switching to '411b1692811b2ecac59cb0df0f920861c7cf179a'.
You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.
If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:
git switch -c <new-branch-name>
Or undo this operation with:
git switch -
Turn off this advice by setting config variable advice.detachedHead to false
HEAD is now at 411b169281 GH-103082: Implementation of PEP 669: Low Impact Monitoring for CPython (GH-103083)
Switched to and reset branch 'main'
In file included from Python/instrumentation.c:9:
./Include/internal/pycore_object.h:20:35: warning: initialization of ‘PyObject *’ {aka ‘struct _object *’} from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
20 | #define _PyObject_IMMORTAL_REFCNT 999999999
| ^~~~~~~~~
Python/instrumentation.c:19:5: note: in expansion of macro ‘_PyObject_IMMORTAL_REFCNT’
19 | _PyObject_IMMORTAL_REFCNT,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
./Include/internal/pycore_object.h:20:35: note: (near initialization for ‘DISABLE._ob_next’)
20 | #define _PyObject_IMMORTAL_REFCNT 999999999
| ^~~~~~~~~
Python/instrumentation.c:19:5: note: in expansion of macro ‘_PyObject_IMMORTAL_REFCNT’
19 | _PyObject_IMMORTAL_REFCNT,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
Python/instrumentation.c:20:5: warning: initialization of ‘PyObject *’ {aka ‘struct _object *’} from incompatible pointer type ‘PyTypeObject *’ {aka ‘struct _typeobject *’} [-Wincompatible-pointer-types]
20 | &PyBaseObject_Type
| ^
Python/instrumentation.c:20:5: note: (near initialization for ‘DISABLE._ob_prev’)
./Include/internal/pycore_object.h:20:35: warning: initialization of ‘PyObject *’ {aka ‘struct _object *’} from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
20 | #define _PyObject_IMMORTAL_REFCNT 999999999
| ^~~~~~~~~
Python/instrumentation.c:25:5: note: in expansion of macro ‘_PyObject_IMMORTAL_REFCNT’
25 | _PyObject_IMMORTAL_REFCNT,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
./Include/internal/pycore_object.h:20:35: note: (near initialization for ‘_PyInstrumentation_MISSING._ob_next’)
20 | #define _PyObject_IMMORTAL_REFCNT 999999999
| ^~~~~~~~~
Python/instrumentation.c:25:5: note: in expansion of macro ‘_PyObject_IMMORTAL_REFCNT’
25 | _PyObject_IMMORTAL_REFCNT,
| ^~~~~~~~~~~~~~~~~~~~~~~~~
Python/instrumentation.c:26:5: warning: initialization of ‘PyObject *’ {aka ‘struct _object *’} from incompatible pointer type ‘PyTypeObject *’ {aka ‘struct _typeobject *’} [-Wincompatible-pointer-types]
26 | &PyBaseObject_Type
| ^
Python/instrumentation.c:26:5: note: (near initialization for ‘_PyInstrumentation_MISSING._ob_prev’)
Modules/gcmodule.c:450: visit_decref: Assertion "!_PyObject_IsFreed(op)" failed
Enable tracemalloc to get the memory block allocation traceback
object address : 0x7f159fa02740
object refcount : 1
object type : 0x563902ad8980
object type name: dict
object repr : make: *** [Makefile:1282: Python/frozen_modules/abc.h] Segmentation fault (core dumped)
find: ‘build’: No such file or directory
find: ‘build’: No such file or directory
find: ‘build’: No such file or directory
find: ‘build’: No such file or directory
make: [Makefile:2676: clean-retain-profile] Error 1 (ignored) |
Sorry, something went wrong.
… CPython (pythonGH-103083) * The majority of the monitoring code is in instrumentation.c * The new instrumentation bytecodes are in bytecodes.c * legacy_tracing.c adapts the new API to the old sys.setrace and sys.setprofile APIs
…here it was removed from the struct. See PEP-669 (https://peps.python.org/pep-0669/) and the implementation in python/cpython#103083. There is more to be done to properly support PEP-669, but this makes it compile. See #5450
| Back | FazBrowse Home | New Git URL |
This implements PEP 669.
There are a couple of things missing, but no harm in early review.