classification
Title: Optimize PyDictObject
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: b@n, inada.naoki, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2018-05-17 10:05 by b@n, last changed 2018-05-18 08:05 by serhiy.storchaka. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 6932 closed python-dev, 2018-05-17 10:17
Messages (8)
msg316901 - (view) Author: (b@n) * Date: 2018-05-17 10:05
make_keys_shared reusing oldkeys memory
msg316903 - (view) Author: Inada Naoki (inada.naoki) * (Python committer) Date: 2018-05-17 10:24
Could you show some micro benchmark shows performance benefit?
msg316906 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-05-17 10:44
Are you aware of causes that prevented writing the code in this way? This will break OrderedDict. Issue31954 is an attempt to solve this problem (and it supersedes this issue).
msg316909 - (view) Author: (b@n) * Date: 2018-05-17 11:10
Will not break OrderedDict, The order is still the same.
msg316911 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-05-17 11:27
Ah, I didn't notice that this affects only dicts with shared keys! Well, this is not related to the issue with OrderedDict which can't have shared keys.

Still we need evidences of the performance benefit. The PR adds >40 lines of code and the benefit should be large enough to compensate the maintaining complexity and possible performance regressions in other parts of the code.
msg316976 - (view) Author: (b@n) * Date: 2018-05-17 18:44
A little performance optimization, but I think the key is not in performance optimization.
The semantics of the dictresize function are not uniform, and it is inconvenient for others to read. The dictresize function should be split to make it just resize. What do you think?
msg316994 - (view) Author: Inada Naoki (inada.naoki) * (Python committer) Date: 2018-05-18 00:56
> A little performance optimization, but I think the key is not in
performance optimization.
> The semantics of the dictresize function are not uniform, and it is
inconvenient for others to read. The dictresize function should be split to
make it just resize. What do you think?

I can't understand.
What dictresize does now other than resize?
Could you show how dictresize can be simplified when clear_dummy_keys() is
added?

Anyway, current my opinion is -1 on this.
We can add similar function when fixing Issue31954.
msg317012 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-05-18 08:05
I'm -1 too. There are no visible benefits, but this change makes maintaining harder and adds a risk of introducing bugs and performance regression.
History
Date User Action Args
2018-05-18 08:05:38serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg317012

stage: patch review -> resolved
2018-05-18 00:56:07inada.naokisetmessages: + msg316994
2018-05-17 18:44:45b@nsetmessages: + msg316976
2018-05-17 11:27:30serhiy.storchakasetmessages: + msg316911
2018-05-17 11:10:52b@nsetmessages: + msg316909
2018-05-17 10:44:14serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg316906
2018-05-17 10:24:08inada.naokisetnosy: + inada.naoki
messages: + msg316903
2018-05-17 10:17:37python-devsetkeywords: + patch
stage: patch review
pull_requests: + pull_request6602
2018-05-17 10:05:44b@ncreate