Author serhiy.storchaka
Recipients Tim Mitchell, christian.heimes, duaneg, ebarry, inada.naoki, larry, ned.deily, rhettinger, serhiy.storchaka, tehybel
Date 2016-11-19.21:33:28
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1479591209.35.0.475177651427.issue27945@psf.upfronthosting.co.za>
In-reply-to
Content
The change to dict_equal() LGTM. It doesn't add an overhead.

For dictiter_iternextitem() I propose other change. It doesn't add an overhead.

There are bugs in the patch for _PyDict_FromKeys().

The change to dictitems_contains() adds an overhead, but it is small and seems unavoidable.

I wondering whether it is worth to use PySequence_Tuple() instead of PySequence_Fast() in PyDict_MergeFromSeq2(). This would add a cost of creating new tuple if items are not tuples, but save increfing/decrefing the key and the value in common case.

I have doubts about insertdict(). Additional incref duplicates increfs in dict_merge(). Is it worth to move it outside of insertdict()?

I have doubts about _PyDict_FromKeys(). It seems to me that it is safe to use _PyDict_Next() in a loop that mutates the dict (not checked _PySet_Next()). With guarded insertdict() additional check is not needed (and it doesn't help against clearing the dict inside insertdict()).

In updated patch fixed some bugs and used different solution for dictiter_iternextitem().
History
Date User Action Args
2016-11-19 21:33:29serhiy.storchakasetrecipients: + serhiy.storchaka, rhettinger, larry, christian.heimes, ned.deily, duaneg, inada.naoki, ebarry, tehybel, Tim Mitchell
2016-11-19 21:33:29serhiy.storchakasetmessageid: <1479591209.35.0.475177651427.issue27945@psf.upfronthosting.co.za>
2016-11-19 21:33:29serhiy.storchakalinkissue27945 messages
2016-11-19 21:33:29serhiy.storchakacreate