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
Remove unicode specialization from set objects #67308
Comments
This tracker item is to record experiments with removing unicode specialization code from set objects and run timings to determine the performance benefits or losses from those specializations.
---------------------- Here are some initial timings for the first patch. It seems to show that intersection benefits slightly and that set creation time is unaffected. $ ./time_suite.sh
100000 loops, best of 3: 14.9 usec per loop
100000 loops, best of 3: 15.3 usec per loop
1000000 loops, best of 3: 1.17 usec per loop
1000000 loops, best of 3: 1.13 usec per loop
10000 loops, best of 3: 24.9 usec per loop
10000 loops, best of 3: 24.2 usec per loop
$ ./time_suite.sh
100000 loops, best of 3: 14.7 usec per loop
100000 loops, best of 3: 14.6 usec per loop
1000000 loops, best of 3: 1.16 usec per loop
1000000 loops, best of 3: 1.07 usec per loop
10000 loops, best of 3: 23.1 usec per loop
10000 loops, best of 3: 23.4 usec per loop
$ ./time_suite.sh
100000 loops, best of 3: 14.5 usec per loop
100000 loops, best of 3: 14.5 usec per loop
1000000 loops, best of 3: 1.16 usec per loop
1000000 loops, best of 3: 1.17 usec per loop
10000 loops, best of 3: 22.5 usec per loop
10000 loops, best of 3: 22 usec per loop |
+1 for this. In my experience sets of unicode keys are not as common as dicts with unicode keys, and the posted numbers make the simplification a no-brainer. |
Attaching an alternative patch that handles the unicode specific case with far less code and less overhead. It seems to speed-up all the timings I've tried. It keeps the unicode_eq() specific path which bypasses several unneeded steps:
|
Timings for no_special_hash.diff: $ ~/cpython/python.exe -m timeit -r7 -s 's={"html"}' '"html" in s'
10000000 loops, best of 7: 0.0315 usec per loop
$ ~/nounicode/python.exe -m timeit -r7 -s 's={"html"}' '"html" in s'
10000000 loops, best of 7: 0.0336 usec per loop
$ ~/cpython/python.exe -m timeit -r7 -s 's={"html"}' '"html" in s'
10000000 loops, best of 7: 0.0316 usec per loop
$ ~/nounicode/python.exe -m timeit -r7 -s 's={"html"}' '"html" in s'
10000000 loops, best of 7: 0.0336 usec per loop
$ ~/cpython/python.exe -m timeit -r7 -s 's={1}' '1 in s'
10000000 loops, best of 7: 0.0331 usec per loop
$ ~/nounicode/python.exe -m timeit -r7 -s 's={1}' '1 in s'
10000000 loops, best of 7: 0.0328 usec per loop
$ ~/cpython/python.exe -m timeit -r7 -s 's={1}' '1 in s'
10000000 loops, best of 7: 0.0332 usec per loop
$ ~/nounicode/python.exe -m timeit -r7 -s 's={1}' '1 in s'
10000000 loops, best of 7: 0.0328 usec per loop The unicode specialization in set_add, set_discard, and set_contains |
+1 for removing unicode specialization. Dictionaries with string keys is a part of the language, but sets of strings are not special. |
I'm not sure I follow: Sets of strings are very common when trying to create a unique set of strings or optimizing "name in set_of_names" lookups. Regarding your benchmark numbers: I have a hard time following how they work. A simply "word in set_of_one_word" certainly doesn't make a good benchmark for these sort of tests :-) You should at least test with sets of various sizes and include the non-matching checks as well. |
This is not nearly so common as attributes or globals access, or passing keyword arguments. |
On 08.01.2015 15:46, Serhiy Storchaka wrote:
Not at the interpreter level, but unlike dicts, sets are not used
(and their associated set operations, i.e. checking for subsets, This is why I don't follow Raymond's reasoning. Of course, if |
@Raymond: Please disable git format for patches, because Rietveld doesn't support such patch and so we don't get the "review" button. |
Without changesets information (not included in the git format) rietveld will try to apply the patch on default and if it applies clearly it will work, so creating the patch against an up to date py3 clone should work even with the git format. |
I'm withdrawing this one. After more work trying many timings on multiple compilers and various sizes and kinds of datasets, it appears that the unicode specialization is still worth it. The cost of the lookup indirection appears to be completely insignificant (i.e. doesn't harm the non-unicode case) while the benefits of the unicode specialized lookup does have measurable benefits in the use case of deduping an iterable of strings. |
On 09.01.2015 09:33, Raymond Hettinger wrote:
Thanks, Raymond, for the additional testing :-) I did a grep over the Python C source code and it seems that sets are The stdlib uses sets with both Unicode strings and integers |
Thanks Mark. I appreciate the effort looking at use cases. I'm reopening this one because I have an alternative patch that simplifies the code but keeps the unicode specialization. It replaces the lookkey indirection with a fast and predictable inline test for PyUnicode_CheckExact. The timings show no measureable difference, but the code is simpler and the setobject size is reduced by one pointer. This simplify future maintenance and development as well as making the code more pleasant. |
May be add the unicode specialization right in PyObject_RichCompareBool? |
That's a possibility but it has much broader ramifications and might or might not be the right thing to do. I'll leave that for someone else to pursue and keep my sights on sets for while. (If you do modify PyObject_RichCompareBool, take a look moving the surrounding incref/decref pair as well). |
New changeset 20f54cdf351d by Raymond Hettinger in branch 'default': |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: