[issue29312] Use FASTCALL in dict.update()

STINNER Victor added the comment: It seems like using FASTCALL would make the code slower, not faster. I close this old issue. -- resolution: -> rejected stage: patch review -> resolved status: open -> closed ___ Python tracker

[issue29312] Use FASTCALL in dict.update()

Jeroen Demeyer added the comment: > but it will make d1.update(**d2) slower with a complexity of O(n): d2 must be > converted to 2 lists This part is still true and it causes a slow-down of about 23% for dict.update(**d), see benchmarks at

[issue29312] Use FASTCALL in dict.update()

Inada Naoki added the comment: > Changing dict.update() calling convention may save a few nanoseconds on > d1.update(d2) call, but it will make d1.update(**d2) way slower with a > complexity of O(n): d2 must be converted to 2 lists (kwnames and args) and > then a new dict should be created.

[issue29312] Use FASTCALL in dict.update()

Jeroen Demeyer added the comment: > d2 must be converted to 2 lists (kwnames and args) and then a new dict should > be created. The last part is not necessarily true. You could do the update directly, without having that intermediate dict. --

[issue29312] Use FASTCALL in dict.update()

STINNER Victor added the comment: Changing dict.update() calling convention may save a few nanoseconds on d1.update(d2) call, but it will make d1.update(**d2) way slower with a complexity of O(n): d2 must be converted to 2 lists (kwnames and args) and then a new dict should be created. I

[issue29312] Use FASTCALL in dict.update()

Change by Jeroen Demeyer : -- pull_requests: +14407 stage: -> patch review pull_request: https://github.com/python/cpython/pull/14589 ___ Python tracker ___

[issue29312] Use FASTCALL in dict.update()

Inada Naoki added the comment: OK, `d1.update(**d2)` is not useful in practice. Practical usages of dict.update() are: * d.update(d2) * d.update([(k1,k2),...]) * d.update(k1=v1, k2=v2, ...) * d.update(**d2, **d3, **d4) # little abuse, but possible. In all of them, kwdict is not used at

[issue29312] Use FASTCALL in dict.update()

Jeroen Demeyer added the comment: You are correct that PyDict_Merge() does not need to recompute the hashes of the keys. However, your example doesn't work because you need string keys for **kwargs. The "str" class caches its hash, so you would need a dict with a "str" subclass as keys to

[issue29312] Use FASTCALL in dict.update()

Inada Naoki added the comment: - d2 = dict(**d1) + d2 = {"fizz": "buzz"} + d2.update(**d1) -- ___ Python tracker ___ ___

[issue29312] Use FASTCALL in dict.update()

Inada Naoki added the comment: > The unpacking is only a problem if you insist on using PyDict_Merge(). It > would be perfectly possible to implement dict merging from a tuple+vector > instead of from a dict. In that case, there shouldn't be a performance > penalty. Really? ``` class K:

[issue29312] Use FASTCALL in dict.update()

Jeroen Demeyer added the comment: > How can we avoid unpacking dict in case of d1.update(**d2)? The unpacking is only a problem if you insist on using PyDict_Merge(). It would be perfectly possible to implement dict merging from a tuple+vector instead of from a dict. In that case, there

[issue29312] Use FASTCALL in dict.update()

Jeroen Demeyer added the comment: Above, I meant #37207 or PR 13930. -- ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue29312] Use FASTCALL in dict.update()

Jeroen Demeyer added the comment: > How can we avoid unpacking dict in case of d1.update(**d2)? We cannot. However, how common is that call? One could argue that we should optimize for the more common case of d1.update(d2). -- ___ Python tracker

[issue29312] Use FASTCALL in dict.update()

Inada Naoki added the comment: How can we avoid unpacking dict in case of d1.update(**d2)? -- ___ Python tracker ___ ___

[issue29312] Use FASTCALL in dict.update()

Jeroen Demeyer added the comment: For the benefit of PR 37207, I would like to re-open this discussion. It may have been rejected for the wrong reasons. Victor's patch was quite inefficient, but that's to be expected: msg285744 mentions a two-step process, but during the discussion the

[issue29312] Use FASTCALL in dict.update()

Roundup Robot added the comment: New changeset e371686229e7 by Victor Stinner in branch 'default': Add a note explaining why dict_update() doesn't use METH_FASTCALL https://hg.python.org/cpython/rev/e371686229e7 -- nosy: +python-dev ___ Python

[issue29312] Use FASTCALL in dict.update()

STINNER Victor added the comment: When analyzing how FASTCALL handles "func(**kwargs)" calls for Python functions, I identified a missed optimization. I created the issue #29318: "Optimize _PyFunction_FastCallDict() for **kwargs". -- resolution: -> rejected

[issue29312] Use FASTCALL in dict.update()

STINNER Victor added the comment: Oops, I forgot a DECREF: fixed in the patch version 2. -- Oh wait, I misunderstood how dict.update() is called. In fact, they are two bytecodes to call a function with keyword arguments. (1) Using **kwargs: >>> def f(): ... d.update(**d2) ... >>>

[issue29312] Use FASTCALL in dict.update()

Raymond Hettinger added the comment: I like the other AC changes to dict in 29311, but this one seems like it shouldn't be done. There is too much twisting around existing code to force it to use AC and the benefit will be almost nothing. dict.update() is mainly used with a list of tuples

[issue29312] Use FASTCALL in dict.update()

INADA Naoki added the comment: Using FASTCALL for methods accepting **kwargs can't skip creating dict in most cases. Because they accepts dict. Even worth, when calling it like `d.update(**config)` (yes, it's abuse of **, but it can be happen in some C methods), FASTCALL may unpack the passed

[issue29312] Use FASTCALL in dict.update()

STINNER Victor added the comment: See also issue #20291: "Argument Clinic should understand *args and **kwargs parameters". -- ___ Python tracker ___

[issue29312] Use FASTCALL in dict.update()

Changes by STINNER Victor : -- components: +Argument Clinic nosy: +larry ___ Python tracker ___

[issue29312] Use FASTCALL in dict.update()

STINNER Victor added the comment: My patch doesn't use _PyStack_AsDict() since this function currently fails with an assertion error if a key is not exactly a string (PyUnicode_CheckExact). dict_update_common() already checks if all keys are string. --

[issue29312] Use FASTCALL in dict.update()

New submission from STINNER Victor: Follow-up of the issue #29311 "Argument Clinic: convert dict methods". The dict.update() method hs a special prototype: def update(arg=None **kw): ... I don't think that Argument Clinic supports it right now, so I propose to first optimize dict_update()