| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
…on creation and destruction of PyCodeObject Co-authored-by: Ye11ow-Flash <janshah@cs.stonybrook.edu>
Co-authored-by: Ye11ow-Flash <janshah@cs.stonybrook.edu>
|
@itamaro I talked this over briefly with @markshannon and it sounds like we should make the same optimization here as @mpage did in 812dd5f for func watchers, to make the check for active watchers a bit more efficient and reduce the overhead on code object creation. |
Sorry, something went wrong.
A bit is set in the bit vector iff there is a watcher set at the corresponding offset in the watcher array. Only notify watchers if at least one bit is set.
|
FYI, I expect to add a PY_CODE_EVENT_INSTRUMENTED event as part of PEP 669, when a code object gets instrumented by monitoring. I would expect tools to treat this new event the same as PyFunction_EVENT_MODIFY_... |
Sorry, something went wrong.
| notify_code_watchers(PyCodeEvent event, PyCodeObject *co) | ||
| { | ||
| PyInterpreterState *interp = _PyInterpreterState_GET(); | ||
| if (interp->active_code_watchers) { |
There was a problem hiding this comment.
I'm OK with this as is, but you could speed this up with an invariant.
(active_code_watchers & (1 << i)) implies interp->code_watchers[i] != NULL
Then the loop can be something like this:
uint8_t watchers = interp->active_code_watchers;
while (watchers) {
int watcher = left_most_bit(watchers);
watchers &= ~(1 << watcher);
if (interp->code_watchers[watcher](...) {
PyErr_WriteUnraisable(...);
}
}
This probably applies to the other watchers as well, so you might want to do this in another PR.
Sorry, something went wrong.
There was a problem hiding this comment.
I think this optimization applies to func watchers, but is already effectively applied for dict and type watchers; in those cases we already never look up the watcher unless the bit is set.
Sorry, something went wrong.
There was a problem hiding this comment.
I have some more watchers-wide cleanup I wanted to do, so will add this optimization to the next PR, thanks!
Sorry, something went wrong.
There was a problem hiding this comment.
cleanup and polish PR (including this optimization) is up at GH-99998
Sorry, something went wrong.
There was a problem hiding this comment.
Looks good. Does this complete the set of watchers?
Sorry, something went wrong.
Yep! (At least the ones we had in mind.) |
Sorry, something went wrong.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️Hi! The buildbot wasm32-emscripten node (pthreads) 3.x has failed when building commit 3c137dc. What do you need to do:
You can take a look at the buildbot page here: https://buildbot.python.org/all/#builders/1050/builds/953 Failed tests:
Summary of the results of the build (if available): == Tests result: FAILURE == 338 tests OK. 10 slowest tests:
1 test failed: 94 tests skipped: Total duration: 20 min 50 sec Click to see traceback logsremote: Enumerating objects: 30, done.
remote: Counting objects: 3% (1/28)
remote: Counting objects: 7% (2/28)
remote: Counting objects: 10% (3/28)
remote: Counting objects: 14% (4/28)
remote: Counting objects: 17% (5/28)
remote: Counting objects: 21% (6/28)
remote: Counting objects: 25% (7/28)
remote: Counting objects: 28% (8/28)
remote: Counting objects: 32% (9/28)
remote: Counting objects: 35% (10/28)
remote: Counting objects: 39% (11/28)
remote: Counting objects: 42% (12/28)
remote: Counting objects: 46% (13/28)
remote: Counting objects: 50% (14/28)
remote: Counting objects: 53% (15/28)
remote: Counting objects: 57% (16/28)
remote: Counting objects: 60% (17/28)
remote: Counting objects: 64% (18/28)
remote: Counting objects: 67% (19/28)
remote: Counting objects: 71% (20/28)
remote: Counting objects: 75% (21/28)
remote: Counting objects: 78% (22/28)
remote: Counting objects: 82% (23/28)
remote: Counting objects: 85% (24/28)
remote: Counting objects: 89% (25/28)
remote: Counting objects: 92% (26/28)
remote: Counting objects: 96% (27/28)
remote: Counting objects: 100% (28/28)
remote: Counting objects: 100% (28/28), done.
remote: Compressing objects: 7% (1/13)
remote: Compressing objects: 15% (2/13)
remote: Compressing objects: 23% (3/13)
remote: Compressing objects: 30% (4/13)
remote: Compressing objects: 38% (5/13)
remote: Compressing objects: 46% (6/13)
remote: Compressing objects: 53% (7/13)
remote: Compressing objects: 61% (8/13)
remote: Compressing objects: 69% (9/13)
remote: Compressing objects: 76% (10/13)
remote: Compressing objects: 84% (11/13)
remote: Compressing objects: 92% (12/13)
remote: Compressing objects: 100% (13/13)
remote: Compressing objects: 100% (13/13), done.
remote: Total 30 (delta 15), reused 15 (delta 15), pack-reused 2
From https://github.com/python/cpython
* branch main -> FETCH_HEAD
Note: switching to '3c137dc613c860f605d3520d7fd722cd8ed79da6'.
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 3c137dc613 GH-91054: Add code object watchers API (GH-99859)
Switched to and reset branch 'main'
../../Objects/obmalloc.c:776:1: warning: always_inline function might not be inlinable [-Wattributes]
776 | arena_map_get(pymem_block *p, int create)
| ^~~~~~~~~~~~~
configure: ../../configure --prefix $(PWD)/target/host --with-pydebug --without-pydebug --with-emscripten-target=node --disable-wasm-dynamic-linking --enable-wasm-pthreads --build=x86_64-pc-linux-gnu --host=wasm32-unknown-emscripten --with-build-python=../build/python
configure: WARNING: using cross tools not prefixed with host triplet
mcc: error: no input files
make: make -j2 all
../../Python/initconfig.c:2347:27: warning: format specifies type 'wint_t' (aka 'int') but the argument has type 'wint_t' (aka 'unsigned int') [-Wformat]
printf(usage_envvars, (wint_t)DELIM, (wint_t)DELIM, PYTHONHOMEHELP);
~~~~~~~~~~~~~ ^~~~~~~~~~~~~
../../Python/initconfig.c:138:18: note: format string is defined here
"PYTHONPATH : '%lc'-separated list of directories prefixed to the\n"
^~~
%c
../../Python/initconfig.c:2347:42: warning: format specifies type 'wint_t' (aka 'int') but the argument has type 'wint_t' (aka 'unsigned int') [-Wformat]
printf(usage_envvars, (wint_t)DELIM, (wint_t)DELIM, PYTHONHOMEHELP);
~~~~~~~~~~~~~ ^~~~~~~~~~~~~
../../Python/initconfig.c:141:58: note: format string is defined here
"PYTHONHOME : alternate <prefix> directory (or <prefix>%lc<exec_prefix>).\n"
^~~
%c
2 warnings generated.
../../Python/pytime.c:297:10: warning: implicit conversion from 'long long' to 'double' changes value from 9223372036854775807 to 9223372036854775808 [-Wimplicit-const-int-float-conversion]
if (!_Py_InIntegralTypeRange(time_t, intpart)) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Include/internal/pycore_pymath.h:72:45: note: expanded from macro '_Py_InIntegralTypeRange'
(_Py_IntegralTypeMin(type) <= v && v <= _Py_IntegralTypeMax(type))
~~ ^~~~~~~~~~~~~~~~~~~~~~~~~
../../Include/internal/pycore_pymath.h:61:88: note: expanded from macro '_Py_IntegralTypeMax'
(_Py_IS_TYPE_SIGNED(type) ? (((((type)1 << (sizeof(type)*CHAR_BIT - 2)) - 1) << 1) + 1) : ~(type)0)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
../../Python/pytime.c:352:14: warning: implicit conversion from 'long long' to 'double' changes value from 9223372036854775807 to 9223372036854775808 [-Wimplicit-const-int-float-conversion]
if (!_Py_InIntegralTypeRange(time_t, intpart)) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Include/internal/pycore_pymath.h:72:45: note: expanded from macro '_Py_InIntegralTypeRange'
(_Py_IntegralTypeMin(type) <= v && v <= _Py_IntegralTypeMax(type))
~~ ^~~~~~~~~~~~~~~~~~~~~~~~~
../../Include/internal/pycore_pymath.h:61:88: note: expanded from macro '_Py_IntegralTypeMax'
(_Py_IS_TYPE_SIGNED(type) ? (((((type)1 << (sizeof(type)*CHAR_BIT - 2)) - 1) << 1) + 1) : ~(type)0)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
../../Python/pytime.c:518:10: warning: implicit conversion from 'long long' to 'double' changes value from 9223372036854775807 to 9223372036854775808 [-Wimplicit-const-int-float-conversion]
if (!_Py_InIntegralTypeRange(_PyTime_t, d)) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Include/internal/pycore_pymath.h:72:45: note: expanded from macro '_Py_InIntegralTypeRange'
(_Py_IntegralTypeMin(type) <= v && v <= _Py_IntegralTypeMax(type))
~~ ^~~~~~~~~~~~~~~~~~~~~~~~~
../../Include/internal/pycore_pymath.h:61:88: note: expanded from macro '_Py_IntegralTypeMax'
(_Py_IS_TYPE_SIGNED(type) ? (((((type)1 << (sizeof(type)*CHAR_BIT - 2)) - 1) << 1) + 1) : ~(type)0)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
3 warnings generated.
../../Modules/expat/xmlparse.c:3116:9: warning: code will never be executed [-Wunreachable-code]
parser->m_characterDataHandler(parser->m_handlerArg, parser->m_dataBuf,
^~~~~~
../../Modules/expat/xmlparse.c:3115:16: note: silence by adding parentheses to mark code as explicitly dead
else if (0 && parser->m_characterDataHandler)
^
/* DISABLES CODE */ ( )
../../Modules/expat/xmlparse.c:4059:9: warning: code will never be executed [-Wunreachable-code]
parser->m_characterDataHandler(parser->m_handlerArg, parser->m_dataBuf,
^~~~~~
../../Modules/expat/xmlparse.c:4058:16: note: silence by adding parentheses to mark code as explicitly dead
else if (0 && parser->m_characterDataHandler)
^
/* DISABLES CODE */ ( )
../../Modules/expat/xmlparse.c:7703:11: warning: format specifies type 'int' but the argument has type 'ptrdiff_t' (aka 'long') [-Wformat]
bytesMore, (account == XML_ACCOUNT_DIRECT) ? "DIR" : "EXP",
^~~~~~~~~
3 warnings generated.
../../Modules/socketmodule.c:4031:33: warning: comparison of integers of different signs: 'unsigned long' and 'long' [-Wsign-compare]
cmsgh != NULL; cmsgh = CMSG_NXTHDR(&msg, cmsgh)) {
^~~~~~~~~~~~~~~~~~~~~~~~
/opt/buildbot/.emscripten_cache/sysroot/include/sys/socket.h:356:44: note: expanded from macro 'CMSG_NXTHDR'
__CMSG_LEN(cmsg) + sizeof(struct cmsghdr) >= __MHDR_END(mhdr) - (unsigned char *)(cmsg) \
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Modules/socketmodule.c:4084:33: warning: comparison of integers of different signs: 'unsigned long' and 'long' [-Wsign-compare]
cmsgh != NULL; cmsgh = CMSG_NXTHDR(&msg, cmsgh)) {
^~~~~~~~~~~~~~~~~~~~~~~~
/opt/buildbot/.emscripten_cache/sysroot/include/sys/socket.h:356:44: note: expanded from macro 'CMSG_NXTHDR'
__CMSG_LEN(cmsg) + sizeof(struct cmsghdr) >= __MHDR_END(mhdr) - (unsigned char *)(cmsg) \
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../Modules/socketmodule.c:4708:54: warning: comparison of integers of different signs: 'unsigned long' and 'long' [-Wsign-compare]
cmsgh = (i == 0) ? CMSG_FIRSTHDR(&msg) : CMSG_NXTHDR(&msg, cmsgh);
^~~~~~~~~~~~~~~~~~~~~~~~
/opt/buildbot/.emscripten_cache/sysroot/include/sys/socket.h:356:44: note: expanded from macro 'CMSG_NXTHDR'
__CMSG_LEN(cmsg) + sizeof(struct cmsghdr) >= __MHDR_END(mhdr) - (unsigned char *)(cmsg) \
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3 warnings generated.
../../Modules/_sqlite/connection.c:2198:19: warning: result of comparison of constant 9223372036854775807 with expression of type 'Py_ssize_t' (aka 'long') is always false [-Wtautological-constant-out-of-range-compare]
if (data->len > 9223372036854775807) { // (1 << 63) - 1
~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~
1 warning generated.
mcc: warning: USE_PTHREADS + ALLOW_MEMORY_GROWTH may run non-wasm code slowly, see https://github.com/WebAssembly/design/issues/1271 [-Wpthreads-mem-growth]
make: *** [Makefile:1860: buildbottest] Error 2
Cannot open file '/opt/buildbot/bcannon-wasm/3.x.bcannon-wasm.emscripten-node-pthreads/build/build/build_oot/host/test-results.xml' for upload |
Sorry, something went wrong.
|
FYI that buildbot failure doesn't seem to be transient. Are there new tests which are recursive and assuming a certain stack depth? Looking at the test failure it seems the call stack is being blown out in the emscripten build (line 515 of https://buildbot.python.org/all/#/builders/1050/builds/953/steps/10/logs/stdio). |
Sorry, something went wrong.
|
There are many buildbots failing to build this PR. Check https://buildbot.python.org/all/#/release_status for reference As per our buildbot policy, this PR will be reverted if the buildbot fleet is not fixed in the next 24h. @markshannon @itamaro can you please take a look? I have opened #99976. I will close it if the issue is fixed before the deadline. |
Sorry, something went wrong.
pythonGH-99859 introduced new buildbot failures, as reported [here](python#91054 (comment)). I was able to reproduce the failures with: ``` ./python.exe -m test -v test_capi.test_watchers -m "*TestCodeObjectWatchers*" -R 3:3 ``` The root cause appears to be to static events counters used in the tests, when running the tests with repetitions (using the same interpreter state), the counts from the first test run affected the next runs. This fixes it by resetting the counts when adding and clearing test watchers.
|
Thanks @pablogsal! I have opened GH-99978 to fix the issue. My local repro passes with the fix - could you get all buildbots to test this PR to confirm? @brettcannon I couldn't find anything in this PR that could trigger deep recursion. I wonder if it's the same issue caught by the other buildbots, but manifesting in a different way on the emscripten buildbot? |
Sorry, something went wrong.
Yup, checking! |
Sorry, something went wrong.
|
@itamaro https://github.com/python/cpython/tree/main/Tools/wasm covers how to do anything WebAssembly-related. |
Sorry, something went wrong.
|
thanks @brettcannon, sorry about the delay, I spent the last two days trying to repro and debug this 😬 TLDR: I think this PR may have pushed something over some threshold that caused the junit xml generation to start hitting stack depth limits. (but is not the root cause of the failures) the steps in the docs failed (basically running ./Tools/wasm/wasm_build.py in the docker container couldn't find the emsdk or something like that), and also didn't apply to the "node (pthreads)" target, which is the one failing. I was able to repro the failure using some mix of the instructions and reverse engineering the buildbot itself. TLDR build and test steps, inside the docker container: mkdir -p build_oot/build build_oot/host
cd build_oot/build
../../configure && make -j
cd ../host
CONFIG_SITE=../../Tools/wasm/config.site-wasm32-emscripten emconfigure ../../configure --prefix '$(PWD)/target/host' --with-pydebug --without-pydebug --with-emscripten-target=node --disable-wasm-dynamic-linking --enable-wasm-pthreads --build=x86_64-pc-linux-gnu --host=wasm32-unknown-emscripten --with-build-python=$(pwd)/../build/python --build=$(../../config.guess) && emmake make -j2 all
make buildbottest 'TESTOPTS=-j2 --junit-xml test-results.xml -j2 ${BUILDBOT_TESTOPTS}' TESTPYTHONOPTS= TESTTIMEOUT=900
this is more or less what the buildbot does, and it fails (not always, but almost always). this also runs the full test suite, but I was able to narrow it down to running just test_capi and still fail: root@18f2ffc1b119:/python-wasm/cpython/build_oot/host# _PYTHON_HOSTRUNNER='/emsdk/node/14.18.2_64bit/bin/node --experimental-wasm-bigint --experimental-wasm-threads --experimental-wasm-bulk-memory' _PYTHON_PROJECT_BASE=/python-wasm/cpython/build_oot/host _PYTHON_HOST_PLATFORM=emscripten-wasm32 PYTHONPATH=/python-wasm/cpython/build_oot/host/build/lib.emscripten-wasm32-3.12:../../Lib _PYTHON_SYSCONFIGDATA_NAME=_sysconfigdata__emscripten_wasm32-emscripten /python-wasm/cpython/build_oot/host/../build/python ../../Tools/scripts/run_tests.py --junit-xml test-results.xml test_capi what makes me think this PR is not the root cause of the regression is that it consistently passes when running just the watchers tests: root@18f2ffc1b119:/python-wasm/cpython/build_oot/host# _PYTHON_HOSTRUNNER='/emsdk/node/14.18.2_64bit/bin/node --experimental-wasm-bigint --experimental-wasm-threads --experimental-wasm-bulk-memory' _PYTHON_PROJECT_BASE=/python-wasm/cpython/build_oot/host _PYTHON_HOST_PLATFORM=emscripten-wasm32 PYTHONPATH=/python-wasm/cpython/build_oot/host/build/lib.emscripten-wasm32-3.12:../../Lib _PYTHON_SYSCONFIGDATA_NAME=_sysconfigdata__emscripten_wasm32-emscripten /python-wasm/cpython/build_oot/host/../build/python ../../Tools/scripts/run_tests.py --junit-xml test-results.xml test_capi.test_watchers and what makes me suspect the root cause is in junit xml generation is that dropping that flag causes test_capi to pass consistently: root@18f2ffc1b119:/python-wasm/cpython/build_oot/host# _PYTHON_HOSTRUNNER='/emsdk/node/14.18.2_64bit/bin/node --experimental-wasm-bigint --experimental-wasm-threads --experimental-wasm-bulk-memory' _PYTHON_PROJECT_BASE=/python-wasm/cpython/build_oot/host _PYTHON_HOST_PLATFORM=emscripten-wasm32 PYTHONPATH=/python-wasm/cpython/build_oot/host/build/lib.emscripten-wasm32-3.12:../../Lib _PYTHON_SYSCONFIGDATA_NAME=_sysconfigdata__emscripten_wasm32-emscripten /python-wasm/cpython/build_oot/host/../build/python ../../Tools/scripts/run_tests.py test_capi I don't know enough about wasm and junit xml to dig deeper for a fix, but I hope this investigation can provide someone who's more familiar with wasm and junit xml with enough information to take it from here! |
Sorry, something went wrong.
|
Thanks for the research! It looks like yesterday some change was made that brought the builder back to being stable. But if it happens again I will definitely come back to your notes to see if we may need to tweak some code in how we're generating that junit XML output. |
Sorry, something went wrong.
Summary: Backport of python/cpython#99859 (plus some more recent changes.) Reviewed By: alexmalyshev Differential Revision: D47201113 fbshipit-source-id: 9390ca289c043bc56ebfdd2ade63305347190a99
| Back | FazBrowse Home | New Git URL |
This PR implements gh-91054, allowing extensions to set a callback function to be notified on creation and destruction of code objects
pyperformance results are 1.00x slower (~neutral) using AWS bare metal machine (full comparison gist)
Co-authored-by: @Ye11ow-Flash