Title: Executors might ignore instrumentation. · Issue #109369 · python/cpython · GitHub
Open Graph Title: Executors might ignore instrumentation. · Issue #109369 · python/cpython
X Title: Executors might ignore instrumentation. · Issue #109369 · python/cpython
Description: Bug report Code handed to the optimizer may not include instrumentation. If instrumentation is added later, the executor does not see it. We remove all ENTER_EXECUTORS when instrumenting, but that doesn't fix the problem of executors tha...
Open Graph Description: Bug report Code handed to the optimizer may not include instrumentation. If instrumentation is added later, the executor does not see it. We remove all ENTER_EXECUTORS when instrumenting, but that ...
X Description: Bug report Code handed to the optimizer may not include instrumentation. If instrumentation is added later, the executor does not see it. We remove all ENTER_EXECUTORS when instrumenting, but that ...
Opengraph URL: https://github.com/python/cpython/issues/109369
X: @github
Domain: github.com
{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Executors might ignore instrumentation.","articleBody":"# Bug report\n\nCode handed to the optimizer may not include instrumentation. If instrumentation is added later, the executor does not see it.\nWe remove all `ENTER_EXECUTORS` when instrumenting, but that doesn't fix the problem of executors that are still running.\n\n### Example\n\nA loop that calls `foo`:\n```Py\nwhile large_number \u003e 0:\n foo()\n large_number -= 1\n```\nThe loop gets turned into an executor, and sometime before `large_number` reaches zero, a debugger gets attached and in some callee of `foo` turns on monitoring of calls. We would expect all subsequent calls to `foo()` to be monitored, but they will not be, as the executor knows nothing about the call to `foo()` being instrumented.\n\n### Let's not rely on the executor/optimizer to handle this.\n\nWe could add a complex de-optimization strategy, so that executors are invalidated when instrumentation occurs, but that is the sort of thing we want to be doing for maximum performance, not for correctness.\n\nIt is much safer, and ultimately no slower (once we implement fancy de-optimization) to add extra checks for instrumentation, so that unoptimized traces are correct.\n\n### The solution\n\nThe solution is quite simple, add a check for instrumentation after every call.\n\nWe can make this less slow (and less simple) by combining the eval-breaker check and instrumentation check into one. This makes the check slightly more complex, but should speed up `RESUME` by less than it slows down every call as every call already has an eval-breaker check.\n\nCombining the two checks reducing the number of bits for versions from 64 to 24 (we need the other bits for eval-breaker, gc, async exceptions, etc). A 64 bit number never overflows (computers don't last long enough), but 24 bit numbers do.\n\nThis will have three main effects, beyond the hopefully very small performance impact for all code:\n* It removes the need for a complete call stack scan of all threads when instrumenting. \n* Tools that set or change monitoring many times will see significant performance improvements as we no longer need to traverse all stacks to re-instrument the whole call stack whenever monitoring changes\n* Tools that set or change monitoring many millions of times will break, as we run out of versions. It is likely that these tools were already broken or had performance so bad as to be totally unusable.\n\n#### Making the check explicit in the bytecode.\n\nWe can simplify all the call instructions by removing the `CHECK_EVAL_BREAKER` check at the end and adding an explicit `RESUME` instruction after every `CALL`\n\nAlthough this has the disadvantage of making the bytecode larger and adding dispatch overhead, it does have the following advantages:\n* Allows tier 1 to optimize the `RESUME` to `RESUME_CHECK` which might be cancel out the additional dispatch overhead.\n* Makes the check explicit, making it feasible for tier 2 optimizations to remove it.\n\n\n\u003c!-- gh-linked-prs --\u003e\n### Linked PRs\n* gh-109846\n* gh-110358\n* gh-110384\n* gh-111642\n* gh-111657\n\u003c!-- /gh-linked-prs --\u003e\n","author":{"url":"https://github.com/markshannon","@type":"Person","name":"markshannon"},"datePublished":"2023-09-13T09:28:41.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":3},"url":"https://github.com/109369/cpython/issues/109369"}
| route-pattern | /_view_fragments/issues/show/:user_id/:repository/:id/issue_layout(.:format) |
| route-controller | voltron_issues_fragments |
| route-action | issue_layout |
| fetch-nonce | v2:c82a535f-480a-68f4-2b87-689a1481d205 |
| current-catalog-service-hash | 81bb79d38c15960b92d99bca9288a9108c7a47b18f2423d0f6438c5b7bcd2114 |
| request-id | E19E:3E22DA:5CB4E1:80036A:696A6F3A |
| html-safe-nonce | 370079822ee46af41f9ab9f057a37abbc375c297586802feca7d5f2e89d795a6 |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiJFMTlFOjNFMjJEQTo1Q0I0RTE6ODAwMzZBOjY5NkE2RjNBIiwidmlzaXRvcl9pZCI6IjI4NzE4MTEwMTY0NDc2NTE2NDIiLCJyZWdpb25fZWRnZSI6ImlhZCIsInJlZ2lvbl9yZW5kZXIiOiJpYWQifQ== |
| visitor-hmac | 14f7dedf951def56286f017ccc86adb5a5eda8d1ba9635e4e657ec26e699017c |
| hovercard-subject-tag | issue:1894124587 |
| github-keyboard-shortcuts | repository,issues,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/_view_fragments/issues/show/python/cpython/109369/issue_layout |
| twitter:image | https://opengraph.githubassets.com/9db1eb73dfc4ed21fc6ed7ea95ee71a9b95ece8be604762e3c4911fafc43aef6/python/cpython/issues/109369 |
| twitter:card | summary_large_image |
| og:image | https://opengraph.githubassets.com/9db1eb73dfc4ed21fc6ed7ea95ee71a9b95ece8be604762e3c4911fafc43aef6/python/cpython/issues/109369 |
| og:image:alt | Bug report Code handed to the optimizer may not include instrumentation. If instrumentation is added later, the executor does not see it. We remove all ENTER_EXECUTORS when instrumenting, but that ... |
| og:image:width | 1200 |
| og:image:height | 600 |
| og:site_name | GitHub |
| og:type | object |
| og:author:username | markshannon |
| hostname | github.com |
| expected-hostname | github.com |
| None | 6fea32d5b7276b841b7a803796d9715bc6cfb31ed549fdf9de2948ac25d12ba6 |
| turbo-cache-control | no-preview |
| go-import | github.com/python/cpython git https://github.com/python/cpython.git |
| octolytics-dimension-user_id | 1525981 |
| octolytics-dimension-user_login | python |
| octolytics-dimension-repository_id | 81598961 |
| octolytics-dimension-repository_nwo | python/cpython |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 81598961 |
| octolytics-dimension-repository_network_root_nwo | python/cpython |
| turbo-body-classes | logged-out env-production page-responsive |
| disable-turbo | false |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | f2d9f6432a5a115ec709295ae70623f33bb80aee |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width