Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1)

#28183: Clean up and speed up dict iteration

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 5 months ago by storchaka+cpython
Modified:
3 years, 5 months ago
Reviewers:
angwerzx, victor.stinner, songofacandy
CC:
rhettinger, haypo, ned.deily, inada.naoki, devnull_psf.upfronthosting.co.za, storchaka, xiang.zhang
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Total comments: 4

Patch Set 3 #

Total comments: 5

Patch Set 4 #

Total comments: 13

Patch Set 5 #

Total comments: 6

Patch Set 6 #

Total comments: 9

Patch Set 7 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Misc/NEWS View 1 2 3 4 5 6 1 chunk +2 lines, -0 lines 0 comments Download
Objects/dictobject.c View 1 2 3 4 5 6 6 chunks +129 lines, -117 lines 0 comments Download

Messages

Total messages: 13
xiang.zhang
http://bugs.python.org/review/28183/diff/18601/Objects/dictobject.c File Objects/dictobject.c (right): http://bugs.python.org/review/28183/diff/18601/Objects/dictobject.c#newcode1755 Objects/dictobject.c:1755: // Py_hash_t hash; Wrong indentation? http://bugs.python.org/review/28183/diff/18601/Objects/dictobject.c#newcode3393 Objects/dictobject.c:3393: PyObject *value ...
3 years, 5 months ago #1
xiang.zhang
3 years, 5 months ago #2
storchaka
Thanks for your review! http://bugs.python.org/review/28183/diff/18601/Objects/dictobject.c File Objects/dictobject.c (right): http://bugs.python.org/review/28183/diff/18601/Objects/dictobject.c#newcode1755 Objects/dictobject.c:1755: // Py_hash_t hash; On 2016/09/18 ...
3 years, 5 months ago #3
victor.stinner_gmail.com
dummy coding style review ;) http://bugs.python.org/review/28183/diff/18686/Objects/dictobject.c File Objects/dictobject.c (right): http://bugs.python.org/review/28183/diff/18686/Objects/dictobject.c#newcode1690 Objects/dictobject.c:1690: int if it's internal, ...
3 years, 5 months ago #4
inada.naoki
http://bugs.python.org/review/28183/diff/18686/Objects/dictobject.c File Objects/dictobject.c (right): http://bugs.python.org/review/28183/diff/18686/Objects/dictobject.c#newcode1690 Objects/dictobject.c:1690: int On 2016/09/28 11:08:28, haypo wrote: > if it's ...
3 years, 5 months ago #5
victor.stinner_gmail.com
http://bugs.python.org/review/28183/diff/18686/Objects/dictobject.c File Objects/dictobject.c (right): http://bugs.python.org/review/28183/diff/18686/Objects/dictobject.c#newcode1690 Objects/dictobject.c:1690: int Oh sorry, I didn't see that the function ...
3 years, 5 months ago #6
victor.stinner_gmail.com
http://bugs.python.org/review/28183/diff/18690/Objects/dictobject.c File Objects/dictobject.c (right): http://bugs.python.org/review/28183/diff/18690/Objects/dictobject.c#newcode1688 Objects/dictobject.c:1688: * to the key and value. Can you please ...
3 years, 5 months ago #7
inada.naoki
http://bugs.python.org/review/28183/diff/18690/Objects/dictobject.c File Objects/dictobject.c (right): http://bugs.python.org/review/28183/diff/18690/Objects/dictobject.c#newcode1688 Objects/dictobject.c:1688: * to the key and value. On 2016/09/28 16:30:34, ...
3 years, 5 months ago #8
victor.stinner_gmail.com
http://bugs.python.org/review/28183/diff/18690/Objects/dictobject.c File Objects/dictobject.c (right): http://bugs.python.org/review/28183/diff/18690/Objects/dictobject.c#newcode3544 Objects/dictobject.c:3544: PyTuple_SET_ITEM(result, 0, key); /* steals reference */ Oh ok. ...
3 years, 5 months ago #9
victor.stinner_gmail.com
http://bugs.python.org/review/28183/diff/18696/Objects/dictobject.c File Objects/dictobject.c (right): http://bugs.python.org/review/28183/diff/18696/Objects/dictobject.c#newcode1744 Objects/dictobject.c:1744: *phash = entry_ptr->me_hash; I checked how _PyDict_Next() is used, ...
3 years, 5 months ago #10
inada.naoki
http://bugs.python.org/review/28183/diff/18696/Objects/dictobject.c File Objects/dictobject.c (right): http://bugs.python.org/review/28183/diff/18696/Objects/dictobject.c#newcode1744 Objects/dictobject.c:1744: *phash = entry_ptr->me_hash; On 2016/09/29 10:57:03, haypo wrote: > ...
3 years, 5 months ago #11
storchaka
http://bugs.python.org/review/28183/diff/18697/Objects/dictobject.c File Objects/dictobject.c (right): http://bugs.python.org/review/28183/diff/18697/Objects/dictobject.c#newcode1002 Objects/dictobject.c:1002: // Shortcut While C99 comments are allowed in new ...
3 years, 5 months ago #12
victor.stinner_gmail.com
3 years, 5 months ago #13
http://bugs.python.org/review/28183/diff/18697/Objects/dictobject.c
File Objects/dictobject.c (right):

http://bugs.python.org/review/28183/diff/18697/Objects/dictobject.c#newcode1009
Objects/dictobject.c:1009: while (_PyDict_Next(dict, &pos, &key, &value, NULL))
{
On 2016/09/29 20:41:44, storchaka wrote:
> value is not use. NULL can be passed.

I proposed to simplify _PyDict_Next() to require to always pass a pointer for
key and value. First I also proposed to also require a pointer for hash, but it
seems like in cases, the hash is not needed.

Here the value was already read in the current code.

http://bugs.python.org/review/28183/diff/18697/Objects/dictobject.c#newcode1010
Objects/dictobject.c:1010: if (!PyUnicode_Check(key)) {
On 2016/09/29 20:41:44, storchaka wrote:
> This is old code. No need to change it. Try to do minimal changes.

Since Naoki modified most of the function, I asked to update the C code for the
PEP 7: {...} for if block.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+