[Python-Dev] PyDict_SetItem hook

[Jeffrey Yasskin]

On Thu, Apr 2, 2009 at 5:57 PM, Guido van Rossum <guido at python.org> wrote:
> On Thu, Apr 2, 2009 at 3:07 PM, Raymond Hettinger <python at rcn.com> wrote:
>>> Wow. Can you possibly be more negative?
>>
>> I think it's worse to give the poor guy the run around
>
> Mind your words please.
>
>> by making him run lots of random benchmarks.  In
>> the end, someone will run a timeit or have a specific
>> case that shows the full effect.  All of the respondents so far seem to have
>> a clear intuition that hook is right in the middle of a critical path.
>>  Their intuition matches
>> what I learned by spending a month trying to find ways
>> to optimize dictionaries.
>>
>> Am surprised that there has been no discussion of why this should be in the
>> default build (as opposed to a compile time option).  AFAICT, users have not
>> previously
>> requested a hook like this.
>
> I may be partially to blame for this. John and Stephan are requesting
> this because it would (mostly) fulfill one of the top wishes of the
> users of Wingware. So the use case is certainly real.
>
>> Also, there has been no discussion for an overall strategy
>> for monitoring containers in general.  Lists and tuples will
>> both defy this approach because there is so much code
>> that accesses the arrays directly.  Am not sure whether the
>> setitem hook would work for other implementations either.
>
> The primary use case is some kind of trap on assignment. While this
> cannot cover all cases, most non-local variables are stored in dicts.
> List mutations are not in the same league, as use case.
>
>> It seems weird to me that Collin's group can be working
>> so hard just to get a percent or two improvement in specific cases for
>> pickling while python-dev is readily entertaining a patch that slows down
>> the entire language.
>
> I don't actually believe that you can know whether this affects
> performance at all without serious benchmarking. The patch amounts to
> a single global flag check as long as the feature is disabled, and
> that flag could be read from the L1 cache.

When I was optimizing the tracing support in the eval loop, we started
with two memory loads and an if test. Removing the whole thing saved
about 3% of runtime, although I think that had been as high as 5% when
Neal measured it a year before. (That indicates that the exact
arrangement of the code can affect performance in subtle and annoying
ways.) Removing one of the two loads saved about 2% of runtime. I
don't remember exactly which benchmark that was; it may just have been
pybench.

Here, we're talking about introducing a load+if in dicts, which is
less critical than the eval loop, so I'd guess that the effect will be
less than 2% overall. I do think the real-life benchmarks are worth
getting for this, but they may not predict the effect after other code
changes. And I don't really have an opinion on what performance hit
for normal use is worth better debugging.

>> If my thoughts on the subject bug you, I'll happily
>> withdraw from the thread.  I don't aspire to be a
>> source of negativity.  I just happen to think this proposal isn't a good
>> idea.
>
> I think we need more proof either way.
>
>> Raymond
>>
>>
>>
>> ----- Original Message ----- From: "Guido van Rossum" <guido at python.org>
>> To: "Raymond Hettinger" <python at rcn.com>
>> Cc: "Thomas Wouters" <thomas at python.org>; "John Ehresman"
>> <jpe at wingware.com>; <python-dev at python.org>
>> Sent: Thursday, April 02, 2009 2:19 PM
>> Subject: Re: [Python-Dev] PyDict_SetItem hook
>>
>>
>> Wow. Can you possibly be more negative?
>>
>> 2009/4/2 Raymond Hettinger <python at rcn.com>:
>>>
>>> The measurements are just a distractor. We all already know that the hook
>>> is being added to a critical path. Everyone will pay a cost for a feature
>>> that few people will use. This is a really bad idea. It is not part of a
>>> thorough, thought-out framework of container hooks (something that would
>>> need a PEP at the very least). The case for how it helps us is somewhat
>>> thin. The case for DTrace hooks was much stronger.
>>>
>>> If something does go in, it should be #ifdef'd out by default. But then, I
>>> don't think it should go in at all.
>>>
>>>
>>> Raymond
>>>
>>>
>>>
>>>
>>> On Thu, Apr 2, 2009 at 04:16, John Ehresman <jpe at wingware.com> wrote:
>>>>
>>>> Collin Winter wrote:
>>>>>
>>>>> Have you measured the impact on performance?
>>>>
>>>> I've tried to test using pystone, but am seeing more differences between
>>>> runs than there is between python w/ the patch and w/o when there is no
>>>> hook
>>>> installed. The highest pystone is actually from the binary w/ the patch,
>>>> which I don't really believe unless it's some low level code generation
>>>> affect. The cost is one test of a global variable and then a switch to
>>>> the
>>>> branch that doesn't call the hooks.
>>>>
>>>> I'd be happy to try to come up with better numbers next week after I get
>>>> home from pycon.
>>>
>>> Pystone is pretty much a useless benchmark. If it measures anything, it's
>>> the speed of the bytecode dispatcher (and it doesn't measure it
>>> particularly
>>> well.) PyBench isn't any better, in my experience. Collin has collected a
>>> set of reasonable benchmarks for Unladen Swallow, but they still leave a
>>> lot
>>> to be desired. From the discussions at the VM and Language summits before
>>> PyCon, I don't think anyone else has better benchmarks, though, so I would
>>> suggest using Unladen Swallow's:
>>> http://code.google.com/p/unladen-swallow/wiki/Benchmarks