bpo-1635741: Enable unicode_release_interned() without insure or valgrind.#21087
bpo-1635741: Enable unicode_release_interned() without insure or valgrind.#21087shihai1991 wants to merge 9 commits into
Conversation
|
After this PR, from |
Sorry, something went wrong.
vstinner
left a comment
There was a problem hiding this comment.
Can you please try to measure the important on performance of this change? You can use:
python3 -m pyperf command -- ./python -S -c pass
Sorry, something went wrong.
Note: Please use a release build ( |
Sorry, something went wrong.
Hi, victor. Got this benchmark result. $ python3 -m pyperf command -- ./python -S -c pass
Try to rerun the benchmark with more runs, values and/or loops. command: Mean +- std dev: 14.3 ms +- 1.6 ms $ python3 -m pyperf command -- ./python -S -c pass --duplicate=100
Try to rerun the benchmark with more runs, values and/or loops. command: Mean +- std dev: 15.1 ms +- 2.3 ms |
Sorry, something went wrong.
What did you measure? What is the reference (unmodified master branch) and what is the modified code (with your PR)? You can store the benchmark result in a file ( |
Sorry, something went wrong.
|
What is the dictionary size of the interned strings at Python exit (in unicode_release_interned())? |
Sorry, something went wrong.
|
Please rebase your PR on master, I modified unicodeobject.c to make singletons per interpreter. |
Sorry, something went wrong.
python3 is master, ./python with tihs PR. I test it again after I fix the assert problem. |
Sorry, something went wrong.
You tested ./python is both cases. Usually, to measure the performance of a change, you need a reference (your "python3") and the modified Python (your "./python"). |
Sorry, something went wrong.
ok, Look like I confused |
Sorry, something went wrong.
Yeah, that would work :-) Anyway, I expect no significant difference. So the most important data point is the number of items in the dictionary at exit. |
Sorry, something went wrong.
Got this result ;) : |
Sorry, something went wrong.
(gdb) p PyDict_Size(interned) |
Sorry, something went wrong.
|
I wrote PR #21269 which clears interned strings in the correct order. |
Sorry, something went wrong.
https://bugs.python.org/issue1635741