GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||
GeezerVison is showing github.com — gh-132657: Improve performance in specialize_dict_access_hint() via adding specialization for PY_GIL_DISABLED by benediktjohannes · Pull Request #144886 · python/cpython Conversation
|
|
This now also refers to PEP 810 and implements a change made in #142351 by @pablogsal, credits to him as well 👍 . |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@benediktjohannes For performance related reviews please create a benchmark. We have to balance the performance improvement to increased complexity and (in this case) memory requirements (deferred objects are handled differently by the gc). |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Hi @eendebakpt, thanks for your answer! I already thought of that comment, but my thought was that a Benchmark seems unnecessary to me in this case because changes like these were already done in another place (see linked PRs). My thought was that this is understandable from looking at the code because I don’t have a testing build installed currently. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
My initial reaction is that this likely increases memory usage too much. The other PR enables deferred reference counting for objects accessed as global variables. It should be a likely that globals typically live for a long time and so it's okay to only free them when the GC runs (which is what happens when you used deferred reference counting). When accessing objects via a dict, I suspect it's more likely those objects have a short lifetime. E.g. you might access it from the dict a number of times in a short time window, enough to enable specialization. However, the dict and the entry might be freed quickly after that. This is all speculation though. You would have to run some benchmarks to see what the effect is. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
This PR is stale because it has been open for 30 days with no activity. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.