[Python-Dev] Nested scopes core dump

[Michael Hudson]

"Tim Peters" <tim.one@home.com> writes:

> [Michael Hudson]
> >>> Maybe you could do the check for resize *after* the call to
> >>> insertdict?  I think that would work, but I wouldn't like to go
> >>> messing with such a performance critical bit of code without some
> >>> careful thinking.
> 
> [Guido]
> >> No, that could still decide to resize, couldn't it?
> 
> [Michael]
> > Yes, but not when you're inserting on a key that is already in the
> > dictionary - because the resize would have happened when the key was
> > inserted into the dictionary, and thus the problem we're seeing here
> > wouldn't happen.
> 
> Careful:  this comment is only half the truth:
> 
> 	/* if fill >= 2/3 size, double in size */

Yes, that could be clearer.  I was confused by the distinction between
ma_used and ma_fill for a bit.

> The dictresize following is also how dicts *shrink*.  That is, build
> up a dict, delete a whole bunch of keys, and nothing at all happens
> to the size until you call setitem again (actually, I think you need
> to call it more than once -- the behavior is tricky).

Well, as I read it, if you delete a bunch of keys and then insert the
same keys again (as in pybench's SimpleDictManipulation), no resize
will happen because ma_fill will be unaffected.  A resize will only
happen if you fill up enough slots to get the 

    mp->ma_fill*3 >= mp->ma_size*2

to trigger.

> In any case, that a key is already in the dict does not guarantee
> that a dict won't resize (via shrinking) when doing a setitem.

Yes.  But I still think that the patch I posted here (the one that
checks for resize after the call to insertdict in PyDict_SetItem)
yesterday will suffice; even if you've deleted a bunch of keys,
ma_fill will be unaffected by the deletes so the size check before the
insertdict won't be triggered (becasue it wasn't by the one after the
call to insertdict in the last call to setitem) and neither will the
size check after the call to insertdict won't be triggered (because
you're inserting on a key already in the dictionary and so ma_fill
will be unchagned).  But this is mighty fragile; something more
explicit is almost certainly a good idea.

So someone should either

> bite the bullet and add a new PyDict_AdjustSize function, just
> duplicating the resize logic.  

or just put a check in PyDict_Next, or outlaw this practice and fix
the places that do it.  And then document the conclusion.  And do it
before 2.1b2 on Friday.  I'll submit a patch, unless you're very
quick.

> Delicate, though.

Uhh, I'd say so.

Cheers,
M.

-- 
 Very clever implementation techniques are required to implement this
 insanity correctly and usefully, not to mention that code written
 with this feature used and abused east and west is exceptionally
 exciting to debug.       -- Erik Naggum on Algol-style "call-by-name"