[Patches] [ python-Patches-1200051 ] Small optimization for PyDict_Merge()

[SourceForge.net]

Patches item #1200051, was opened at 2005-05-11 13:02
Message generated for change (Comment added) made by rhettinger
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1200051&group_id=5470

Category: Core (C code)
Group: Python 2.5
>Status: Closed
>Resolution: Accepted
Priority: 5
Submitted By: Barry A. Warsaw (bwarsaw)
Assigned to: Raymond Hettinger (rhettinger)
Summary: Small optimization for PyDict_Merge()

Initial Comment:
This patch avoids repeated PyDict_GetItem() calls when
'a' in PyDict_Merge() is known to be empty. 
Optimization found by Matt Messier.

This could also be applied to Python 2.5.

----------------------------------------------------------------------

>Comment By: Raymond Hettinger (rhettinger)
Date: 2005-05-14 13:08

Message:
Logged In: YES 
user_id=80475

Applied as Objects/dictobject.c  2.165

----------------------------------------------------------------------

Comment By: Barry A. Warsaw (bwarsaw)
Date: 2005-05-12 12:38

Message:
Logged In: YES 
user_id=12800

Thanks for the clarification.  It happens in our code, but I
can't say I've done much benchmarking.

----------------------------------------------------------------------

Comment By: Raymond Hettinger (rhettinger)
Date: 2005-05-12 11:41

Message:
Logged In: YES 
user_id=80475

Right.  I marked it for 2.5 because optimizations usually do
not get backported.

With respect to Tim's comment about slowing the common case,
I don't see it.  The test is outside the inner loop.  The
variable that it sets helps short-circuit the inner-loop
tests.  All-in-all, it's a net win.

With respect to measurement, I would be surprised if the OP
could easily demonstrate a speed-up.  Grepping through
Objects and Modules shows zero calls to PyDict_Merge() where
the override argument is zero.   The idea looks sound, but
it looks like you're optimizing a case the doesn't occur in
practice.

----------------------------------------------------------------------

Comment By: Tim Peters (tim_one)
Date: 2005-05-12 11:03

Message:
Logged In: YES 
user_id=31435

Well, in general we don't backport optimizations:  that 
something _could_ be made faster is rarely considered to 
be "a bug".  If, e.g., this was very much faster in 2.3 than in 
2.4, then _maybe_ it could be sold as a 2.4 bug.  Otherwise 
presumption favors that it's just one more of the literally 
thousands of speedups that could always be made.

In this case, it's not a pure win, so is especially dubious for a 
bugfix release:  it adds a test + taken-branch to the critical 
path whenever the function is called with a _non_-empty 
target dict.  So it slows what's probably the most common 
case a little, in order to speed a probably-rarer case.  Is that 
a net win or net loss?  Depends on the app, I suppose.

Did you try this in your app and _measure_ a net win?

----------------------------------------------------------------------

Comment By: Barry A. Warsaw (bwarsaw)
Date: 2005-05-12 09:45

Message:
Logged In: YES 
user_id=12800

Does changing the group mean you don't think this should be
applied to 2.4?  It doesn't change semantics at all.  (I'm
not necessarily disagreeing with you, but am just asking for
some justification in the issue tracker.)

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1200051&group_id=5470