[issue13521] Make dict.setdefault() atomic

Antoine Pitrou added the comment: Thank you Filip! I've now committed the patch. -- resolution: -> fixed stage: patch review -> committed/rejected status: open -> closed ___ Python tracker ___

[issue13521] Make dict.setdefault() atomic

Roundup Robot added the comment: New changeset 90572ccda12c by Antoine Pitrou in branch '3.2': Issue #13521: dict.setdefault() now does only one lookup for the given key, making it "atomic" for many purposes. http://hg.python.org/cpython/rev/90572ccda12c New changeset 3dfa98cf2e26 by Antoine P

[issue13521] Make dict.setdefault() atomic

Roundup Robot added the comment: New changeset 5c52e7c6d868 by Antoine Pitrou in branch '2.7': Issue #13521: dict.setdefault() now does only one lookup for the given key, making it "atomic" for many purposes. http://hg.python.org/cpython/rev/5c52e7c6d868 -- nosy: +python-dev _

[issue13521] Make dict.setdefault() atomic

Filip Gruszczyński added the comment: Fixed both issues. -- Added file: http://bugs.python.org/file24652/13521_27_4.patch ___ Python tracker ___

[issue13521] Make dict.setdefault() atomic

Antoine Pitrou added the comment: > No more double lookup. Your patch doesn't check hashed2.eq_count. Since the dict specification doesn't say on which instance __eq__ will be called when doing a lookup, the patch should either check ``hashed1.eq_count + hashed2.eq_count``, or make eq_count a c

[issue13521] Make dict.setdefault() atomic

Filip Gruszczyński added the comment: No more double lookup. -- Added file: http://bugs.python.org/file24244/13521_27_3.patch ___ Python tracker ___

[issue13521] Make dict.setdefault() atomic

Filip Gruszczyński added the comment: Thanks, I'll try that. Like before I will probably find time next weekend. -- ___ Python tracker ___ __

[issue13521] Make dict.setdefault() atomic

Antoine Pitrou added the comment: > Do you have any idea, how I might this part? I mean how to check, that > this is called only once? Checking that __eq__ is called only once should be a good proxy. -- ___ Python tracker

[issue13521] Make dict.setdefault() atomic

Filip Gruszczyński added the comment: I understand you are talking about this call: mp->ma_lookup(mp, key, hash); I haven't noticed that earlier. I'll try to provide a better fix (for 2.7 first, after we agree it's good enough, I will provide one for 3.3). Do you have any idea, how I might t

[issue13521] Make dict.setdefault() atomic

Antoine Pitrou added the comment: Looking at the patch again, I think this isn't enough. setdefault() will still call the lookup routine twice which, in the general case (i.e. lookdict() not lookdict_unicode()), can call arbitrary Python code through e.g. __eq__ methods. -- _

[issue13521] Make dict.setdefault() atomic

Filip Gruszczyński added the comment: Bump! There was no activity here for two weeks. Is my patch for 2.7 ok or should I do something more about it? -- ___ Python tracker ___ _

[issue13521] Make dict.setdefault() atomic

Filip Gruszczyński added the comment: Patch for 2.7. -- Added file: http://bugs.python.org/file24035/13521_27_1.patch ___ Python tracker ___

[issue13521] Make dict.setdefault() atomic

Raymond Hettinger added the comment: Thanks, I'll look at it over the next few days. -- ___ Python tracker ___ ___ Python-bugs-list m

[issue13521] Make dict.setdefault() atomic

Antoine Pitrou added the comment: > I would like to continue working on this test, but incoming week is > pretty intensive for me. Is it ok if I returned to working on this > during the weekend? Of course. There's no rush. (I'm really not sure it should go in 2.7 and 3.2; this looks more like a

[issue13521] Make dict.setdefault() atomic

Filip Gruszczyński added the comment: I have tried to port patch to python2.7, but apparently I must be doing something wrong, because while most tests pass, several fail (including tests for unittest itself). I would like to continue working on this test, but incoming week is pretty intensi

[issue13521] Make dict.setdefault() atomic

Raymond Hettinger added the comment: Let's just start with a working 2.7 patch and go from there. -- ___ Python tracker ___ ___ Pytho

[issue13521] Make dict.setdefault() atomic

Filip Gruszczyński added the comment: I'll send a patch, when I get home from work. -- ___ Python tracker ___ ___ Python-bugs-list ma

[issue13521] Make dict.setdefault() atomic

Antoine Pitrou added the comment: > Well, if this is going to be applied to 2.7, "nonlocal" is invalid > syntax there. Ah, good point. -- ___ Python tracker ___ ___

[issue13521] Make dict.setdefault() atomic

Jesús Cea Avión added the comment: Well, if this is going to be applied to 2.7, "nonlocal" is invalid syntax there. I guess that being able to use the same test in 2.7 and 3.2/3.3 would be nice. Style, but justified. -- ___ Python tracker

[issue13521] Make dict.setdefault() atomic

Filip Gruszczyński added the comment: I haven't given it much thought, when I was making the choice of using nonlocal rather than self.count. I was rather excited to see, if the change will work as I wanted it to. If you believe it would be better to use an attribute, I'll be happy to change

[issue13521] Make dict.setdefault() atomic

Antoine Pitrou added the comment: > Filip: Is there a reasons for using a nonlocal count rather than e.g. > self.count? Otherwise the test looks good. Eric, please, could we stop such pointless nitpicking about one's stylistic preferences? "nonlocal" is as good as any other solution here, and

[issue13521] Make dict.setdefault() atomic

Éric Araujo added the comment: Thanks for the idea Jesús, even though I didn’t get the change to use it :) Filip: Is there a reasons for using a nonlocal count rather than e.g. self.count? Otherwise the test looks good. -- ___ Python tracker

[issue13521] Make dict.setdefault() atomic

Antoine Pitrou added the comment: The patch looks ok to me. I'll let Raymond make the final call. -- ___ Python tracker ___ ___ Pytho

[issue13521] Make dict.setdefault() atomic

Filip Gruszczyński added the comment: Done. -- Added file: http://bugs.python.org/file23918/13521_2.patch ___ Python tracker ___ ___

[issue13521] Make dict.setdefault() atomic

Antoine Pitrou added the comment: If _PyDict_SetItemUsingHash is module-private, it should be declared "static". Also, better if it follows the usual naming of static functions inside that C file (i.e. "dict_some_lowercase_name"). -- stage: needs patch -> patch review ___

[issue13521] Make dict.setdefault() atomic

Filip Gruszczyński added the comment: Also: I'll be happy to work further on this patch, if I get some comments and advice. -- ___ Python tracker ___ __

[issue13521] Make dict.setdefault() atomic

Filip Gruszczyński added the comment: I have written a patch and a test, but since it's changing C code, I am far from being sure if it's achieve the expected behavior in the right way. There are also tests and running whole test suite didn't bring any errors. -- keywords: +patch nosy

[issue13521] Make dict.setdefault() atomic

Jesús Cea Avión added the comment: Eric, overload "__hash__()" and check that is only called once, while now would be called twice. -- ___ Python tracker ___ __

[issue13521] Make dict.setdefault() atomic

Éric Araujo added the comment: maniram: We try to keep bug reports focused on one thing, and we don’t generally collect votes (we prefer that people vote with patches, tests and messages with content—for example, saying exactly what “more robust” should be). Here I think Antoine and Jesús sa

[issue13521] Make dict.setdefault() atomic

maniram maniram added the comment: +1 for atomic and more robust -- nosy: +maniram.maniram ___ Python tracker ___ ___ Python-bugs-lis

[issue13521] Make dict.setdefault() atomic

Changes by John O'Connor : -- nosy: +jcon ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.or

[issue13521] Make dict.setdefault() atomic

Jesús Cea Avión added the comment: Atomic and faster... +1. -- nosy: +jcea ___ Python tracker ___ ___ Python-bugs-list mailing list U

[issue13521] Make dict.setdefault() atomic

Changes by Meador Inge : -- nosy: +meador.inge stage: -> needs patch ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubsc

[issue13521] Make dict.setdefault() atomic

Changes by Raymond Hettinger : -- assignee: -> rhettinger ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http

[issue13521] Make dict.setdefault() atomic

Antoine Pitrou added the comment: +1. -- nosy: +pitrou ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://

[issue13521] Make dict.setdefault() atomic

New submission from Raymond Hettinger : A dialog on python-dev suggests that dict.setdefault() was intended to be atomic and that a number of experienced developers have deployed code relying on its atomicity: http://mail.python.org/pipermail/python-3000/2007-July/008693.html The actual impl