Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-103092: Isolate _decimal#103381
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gh-103092: Isolate _decimal#103381
Uh oh!
There was an error while loading. Please reload this page.
Conversation
CharlieZhao95 commented Apr 8, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
erlend-aasland commented Apr 8, 2023
I'm not sure I follow;
Can you please spell this out? We cannot guess what you mean by this :) |
CharlieZhao95 commented Apr 9, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Background When I executed Traceback (most recent call last): File "F:\CPython\github\cpython\Lib\test\test_decimal.py", line 5900, in<module> test_main(arith=True, verbose=True) File "F:\CPython\github\cpython\Lib\test\test_decimal.py", line 5840, in test_main init(C) File "F:\CPython\github\cpython\Lib\test\test_decimal.py", line 110, in init DefaultTestContext = m.Context( ^^^^^^^^^^ KeyError: 'invalid signal dict'I found that the place where the error occurs is PyDict_GetItemWithError(val, cm->ex) in So I guess I'm not sure if these works are necessary, please correct me if I'm wrong :)
Of course, there is currently some unclean code. For example, |
erlend-aasland commented Apr 9, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Is this code added by this PR, or code that is already there? For the latter, please do not include such changes in this PR. Instead, create an issue (unless there already is one), explain what you intend to do, and create a separate PR. This PR should focus on isolating |
CharlieZhao95 commented Apr 10, 2023
Sorry for not being clear. I mean the code added by this PR. I like to leave "FIXME" in my code to remind myself of potential work :) |
CharlieZhao95 commented Apr 12, 2023
I tested the reference counts using erlend's script and they look well. $./python measure.py before=90491, after=93999 before=94000, after=94000 before=94000, after=94000 before=94000, after=94000 before=94000, after=94000Also, I compared the execution time of the |
CharlieZhao95 commented Apr 12, 2023
The code that I can think of that may need to be improved is as follows:
|
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
bedevere-bot commented Jun 1, 2023
🤖 New build scheduled with the buildbot fleet by @kumaraditya303 for commit 58f0049 🤖 If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again. |
erlend-aasland commented Jun 2, 2023
I'm sorry, but I don't have the bandwidth to review this in the near future. |
| intn, mem; | ||
| assert(PyDecContext_Check(self)); | ||
| decimal_state*state=get_module_state_by_def(Py_TYPE(self)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
guard with Py_DEBUG
erlend-aasland commented Jun 16, 2023
IMO, we should break this up into multiple PRs, like we did with |
CharlieZhao95 commented Jun 20, 2023
Thanks. This PR made a lot of changes at the same time, which might make it difficult to review. But the I plan to break the PR into the following parts:
Potential work: Add Argument Clinic (It might be worth opening a new issue) And I noticed that @kumaraditya303 has already done some review work, what do you think? |
kumaraditya303 commented Jun 21, 2023
SGTM, ping me when you are done, |
This PR is based on @erlend-aasland 's great work. Currently, most of the test cases can be passed, but still some work needs to be done. I make this PR as a draft so anyone can remind me of missing work or improve this code.
TODO
cond_mapto heap type (not recorded inglobal-to-fix, but it seems like we should handle it)