gh-132657: Improve performance in specialize_dict_access_hint() via adding specialization for PY_GIL_DISABLED#144886
gh-132657: Improve performance in specialize_dict_access_hint() via adding specialization for PY_GIL_DISABLED#144886benediktjohannes wants to merge 5 commits intopython:mainfrom
Conversation
…alization for PY_GIL_DISABLED
|
This now also refers to PEP 810 and implements a change made in #142351 by @pablogsal, credits to him as well 👍 . |
|
@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). |
|
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. |
|
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. |
General information
This PR aims to improve performance in specialize_dict_access_hint() via adding specialization for PY_GIL_DISABLED (refering to #142843 and #132657 by @eendebakpt and @nascheme, credits to them 👍).
Background
We apply deferred reference counting to the value when the object is owned by a different thread.
Further information
I've not tested and benchmarked this, it's just a code review, but I'm pretty confident that this is correct because it refers to #142843 and basically implements a similar change, but please correct me if I'm mistaken.