[issue28580] Optimize iterating split table values

Changes by Donald Stufft : -- pull_requests: +1015 ___ Python tracker ___ ___

[issue28580] Optimize iterating split table values

Xiang Zhang added the comment: Thanks! -- ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue28580] Optimize iterating split table values

Changes by INADA Naoki : -- resolution: -> fixed stage: commit review -> resolved status: open -> closed ___ Python tracker ___

[issue28580] Optimize iterating split table values

Roundup Robot added the comment: New changeset 3904194d06e6 by INADA Naoki in branch 'default': Issue #28580: Optimize iterating split table values. https://hg.python.org/cpython/rev/3904194d06e6 -- nosy: +python-dev ___ Python tracker

[issue28580] Optimize iterating split table values

INADA Naoki added the comment: LGTM. I'll commit. -- ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue28580] Optimize iterating split table values

Serhiy Storchaka added the comment: v4 LGTM. -- ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue28580] Optimize iterating split table values

Changes by Xiang Zhang : Added file: http://bugs.python.org/file45332/iterate_splittable_v4.patch ___ Python tracker ___

[issue28580] Optimize iterating split table values

Serhiy Storchaka added the comment: In current code there is no UB in _PyDict_Next(). -- ___ Python tracker ___

[issue28580] Optimize iterating split table values

Xiang Zhang added the comment: > I would suggest just remove assert() from your patch and address undefined > behavior in other issue. That's what v2 does. If there is another issue, let's also leave _PyDict_Next to it. -- ___ Python tracker

[issue28580] Optimize iterating split table values

Xiang Zhang added the comment: Currently dict iterator does not allow size changed during iteration. This is more strict than list iterator but still allow modification during iteration. Maybe we could deny all modification by checking dict->ma_version_tag. But that's irrelevant to this

[issue28580] Optimize iterating split table values

Serhiy Storchaka added the comment: It is appropriate if iterating modifying dict raises RuntimeError, produces less items or even produce some items multiple times. What is not appropriate -- crash, hang and infinite iteration. There was a proposition about making this behavior more

[issue28580] Optimize iterating split table values

Xiang Zhang added the comment: Hmm, the resolution could be simple. But how about >>> d = dict.fromkeys(range(100)) >>> for k in range(98): ... del d[k] ... >>> it = iter(d) >>> next(it) 98 >>> d.clear() >>> d[0] = 1 >>> d[1] = 2 >>> next(it) Traceback (most recent call last): File "",

[issue28580] Optimize iterating split table values

Serhiy Storchaka added the comment: Following example causes a crash in debug build: d = dict.fromkeys(range(100)) for k in range(99): del d[k] it = iter(d) assert next(it) == 99 d.clear() d[0] = None next(it) -- ___ Python tracker

[issue28580] Optimize iterating split table values

Changes by Xiang Zhang : Added file: http://bugs.python.org/file45330/iterate_splittable_v3.patch ___ Python tracker ___

[issue28580] Optimize iterating split table values

INADA Naoki added the comment: (Off topic note) For readability, I prefer this style: PyDictKeyEntry *entries = DK_ENTRIES(mp->ma_keys); while (i < n && entries[i].me_value == NULL) i++; But because sizeof(PyDictKeyEntry) is not 2^n, entries[i].me_value, i++ may be slower than

[issue28580] Optimize iterating split table values

Serhiy Storchaka added the comment: I believe the patch doesn't makes iterating split table slower and likely makes it a little faster. The patch adds one additional check, but it removes other checks. Thus this still is an optimization. But the last patch has an undefined behavior.

[issue28580] Optimize iterating split table values

INADA Naoki added the comment: LGTM again for iterate_splittable_v2.patch. How about issue title (and commit message) to "Cleanup iterating split table"? -- ___ Python tracker

[issue28580] Optimize iterating split table values

INADA Naoki added the comment: > Xiang Zhang added the comment: > > Yes, that's the point. I thought to expose an API in testcapimodule for test, > but actually I am not willing to do that since I don't believe this patch > could bring any visible performance change. > I agree with you. I

[issue28580] Optimize iterating split table values

Xiang Zhang added the comment: Yes, that's the point. I thought to expose an API in testcapimodule for test, but actually I am not willing to do that since I don't believe this patch could bring any visible performance change. -- ___ Python tracker

[issue28580] Optimize iterating split table values

Serhiy Storchaka added the comment: There is not easy to benchmark PyDict_Next. dict.__repr__ does much hard work (calling repr() for keys and values, formatting strings). Using _testcapi.test_dict_iteration perhaps is the easiest way to measure the performance of pure PyDict_Next, but it

[issue28580] Optimize iterating split table values

Xiang Zhang added the comment: python -> unpatched, python3 -> patched iterkeys: (split) ./python3 -m perf timeit --compare-to /home/angwer/cpython/python -s 'from argparse import Namespace; ns = Namespace(); [setattr(ns, str(i), str(i)) for i in range(1)]' 'list(iter(ns.__dict__))'

[issue28580] Optimize iterating split table values

Serhiy Storchaka added the comment: Could you please check how this change affects the performance? -- ___ Python tracker ___

[issue28580] Optimize iterating split table values

Xiang Zhang added the comment: Update the patch to use more obvious comparison way. This uses INADA's suggestion and make the code more like other places. I think the performance issue is better to be discussed in #28397. It doesn't have a significant impact on this patch. Hope we can move

[issue28580] Optimize iterating split table values

Serhiy Storchaka added the comment: LGTM. -- stage: patch review -> commit review ___ Python tracker ___ ___

[issue28580] Optimize iterating split table values

Xiang Zhang added the comment: Here is the new patch to apply the optimization to more places. -- Added file: http://bugs.python.org/file45312/iterate_splittable.patch ___ Python tracker

[issue28580] Optimize iterating split table values

Xiang Zhang added the comment: > Xiang, would you update patch? Working on it. -- title: Optimize _PyDict_Next for split table -> Optimize iterating split table values ___ Python tracker