| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Sorry, something went wrong.
|
Hi @colesbury , Please help take a review and correct me if I misunderstand anything about this case. 😊 Wish you a good day. Thanks very much! Best Regards, |
Sorry, something went wrong.
There was a problem hiding this comment.
Could you also run the pyperformance benchmarks on this PR? Deferred reference counting can damage single-threaded code.
Sorry, something went wrong.
| @@ -0,0 +1 @@ | |||
| Make ``type_setattro`` use defer refcount in free-threading for functions without defer refcount. | |||
There was a problem hiding this comment.
This is too technical for a news entry. Instead, can we say something like "Improve multithreaded scaling of dataclasses on the free-threaded build"?
Sorry, something went wrong.
There was a problem hiding this comment.
Hi @ZeroIntensity ,
Thanks very much for your suggestion!
I will update the news entry as suggested.
Best Regards,
Edward
Sorry, something went wrong.
| #ifdef Py_GIL_DISABLED | ||
| if (value != NULL && PyFunction_Check(value)) { | ||
| if (!_PyObject_HasDeferredRefcount(value)) { | ||
| BEGIN_TYPE_LOCK(); |
There was a problem hiding this comment.
We don't need to hold the type lock here, since PyUnstable_Object_EnableDeferredRefcount is thread-safe.
Sorry, something went wrong.
There was a problem hiding this comment.
Hi @ZeroIntensity ,
Thanks very much for pointing this out. ❤
I go through the code of PyUnstable_Object_EnableDeferredRefcount and find that it's thread-safe.
Lines 2734 to 2739 in 3d14805
It uses compare-and-set to guard the _PyGC_BITS_DEFERRED bit flag before updating the payload of the deferred refcount. So only one thread could make this change.
I removed the TYPE_LOCK and the double check of the _PyObject_HasDeferredRefcount. They are unnecessary.
Best Regards,
Edward
Sorry, something went wrong.
| if (value != NULL && PyFunction_Check(value)) { | ||
| if (!_PyObject_HasDeferredRefcount(value)) { | ||
| BEGIN_TYPE_LOCK(); | ||
| if (!_PyObject_HasDeferredRefcount(value)) { |
There was a problem hiding this comment.
Why is this checked twice?
Sorry, something went wrong.
There was a problem hiding this comment.
Hi @ZeroIntensity ,
I tried to double-check the flag to reduce the conflict of entering the lock.
But as you mentioned above, we don't need a lock for PyUnstable_Object_EnableDeferredRefcount .
This double check has been removed with the lock.
An atomic action on the type flag is very efficient and is safe for our scenario.
Thanks very much for your suggestion!
Best Regards,
Edward
Sorry, something went wrong.
Hi @ZeroIntensity , Got it! 😊 Here is my perf report on this PR and main cc6b62a . https://gist.github.com/LindaSummer/8d3420b10bf591b0e1d76336787e0a49 Best Regards, |
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM w/ some minor formatting adjustments
Sorry, something went wrong.
|
Thanks @LindaSummer for the PR, and @colesbury for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14. |
Sorry, something went wrong.
|
Sorry, @LindaSummer and @colesbury, I could not cleanly backport this to 3.14 due to a conflict. cherry_picker ce791541769a41beabec0f515cd62e504d46ff1c 3.14 |
Sorry, something went wrong.
…issue (pythongh-141596) The dataclasses `__init__` function is generated dynamically by a call to `exec()` and so doesn't have deferred reference counting enabled. Enable deferred reference counting on functions when assigned as an attribute to type objects to avoid reference count contention when creating dataclass instances. (cherry picked from commit ce79154) Co-authored-by: Edward Xu <xuxiangad@gmail.com>
|
GH-141750 is a backport of this pull request to the 3.14 branch. |
Sorry, something went wrong.
…h-141596) (gh-141750) The dataclasses `__init__` function is generated dynamically by a call to `exec()` and so doesn't have deferred reference counting enabled. Enable deferred reference counting on functions when assigned as an attribute to type objects to avoid reference count contention when creating dataclass instances. (cherry picked from commit ce79154) Co-authored-by: Edward Xu <xuxiangad@gmail.com>
…ythongh-141596) The dataclasses `__init__` function is generated dynamically by a call to `exec()` and so doesn't have deferred reference counting enabled. Enable deferred reference counting on functions when assigned as an attribute to type objects to avoid reference count contention when creating dataclass instances.
…ythongh-141596) The dataclasses `__init__` function is generated dynamically by a call to `exec()` and so doesn't have deferred reference counting enabled. Enable deferred reference counting on functions when assigned as an attribute to type objects to avoid reference count contention when creating dataclass instances.
| Back | FazBrowse Home | New Git URL |
Issue
#139103
Proposed Changes
Comment
Test Case
Here is my original test case for this problem.
Here is the output for this testcase.
configure command: ./configure --disable-gil "CC=clang" on 4ceb077 .
Root Cause
In test_3, I use _testcapi.pyobject_enable_deferred_refcount to make the generated __init__ with deferred refcount, and the problem is fixed.
Final Action
So I use the PyUnstable_Object_EnableDeferredRefcount provided in _testcapi.pyobject_enable_deferred_refcount to make the function object a deferred refcount one.
cpython/Modules/_testcapi/object.c
Lines 127 to 132 in ed81baf
Here is the output of my test script in this PR.
Here is the result of the new benchmark case in the current PR.
We could see that the instantiate_dataclass is 5.1x faster.
Please correct me if I misunderstand anything about this case.
Thanks very much! 😊
Best Regards,
Edward Xu