[Tres Seaver] [...]

- -1. Python's 'hasattr' semantics are *broken by definition*, and will never be fixed (because of backward compatibility). Non-Zope Python programmers will *not* expect or want exceptions raised from 'hasattr'.

As a local patch, this isn't too bad (one could even package it as a do-nothing-after-initialization product). However, no redistributed product code should rely on the presence of a patched 'hasattr', but should use the 3 argument getattr instead.

OTOH, defining & importing a utility function-- say, safehasattr() --would make it all explicit. That's what ZODB does. I certainly agree that changing the behavior of a builtin function by magic is at best highly dubious practice.

On Fri, May 27, 2005 at 09:25:58AM -0400, Jim Fulton wrote:

Tim Peters wrote:

...

OTOH, defining & importing a utility function-- say, safehasattr() --would make it all explicit. That's what ZODB does.

OK. (BTW, I just went grepping for this safehasattr() in zope 2.7.6's ZODB and didn't find anything. What's it called?) There are ~700 calls to hasattr() currently in the Zope core (ouch!). Are there cases where the current use of hasattr() is considered safe? Or since it's "broken by design", should we replace all 700 calls with this hypothetical safe_hasattr()? While we're on the subject, the other day Dieter Maurer was complaining that: """ An incredibly long time ago, I filed a feature request for "hasattr_unacquired" -- together with patch, unit tests and documentation update. I am convinced that such a function in the DTML namespace (and therefore always available in restricted code) would be much clearer than the "aq_inner.aq_explicit" dance. But, unfortunately, the Zope developers decided not to accept my patch or the "hasattr_unacquired" idea and instead made "aq_inner" accessible by untrusted code. A bad decision! As a consequence, you see the nasty code. """ So, assuming we add a safe_hasattr() function, maybe it could take an extra keyword arg? something like: def safe_hasattr(obj, attr, acquired=True, _marker=[]): if not acquired: obj = aq_inner(aq_explicit(obj)) return getattr(obj, attr, _marker) is not _marker

...

I certainly agree that changing the behavior of a builtin function by magic is at best highly dubious practice.

In fact, consider this vetoed for the Zope core. :)

Fine, that nicely reduces the scope of the discussion :-) -- Paul Winkler http://www.slinkp.com

On Fri, May 27, 2005 at 11:49:03AM -0400, Jim Fulton wrote:

BTW, I prefer to code things like this in the following way:

def safe_hasattr(): marker = object()

def safe_hasattr(obj, attr): return getattr(obj, attr, marker) is not marker

return safe_hasattr

safe_hasattr = safe_hasattr()

This way, there aren't any extra arguments or funny globals to confuse matters. (I've seen code that broke because people tried to reuse marker definitions accross modules.

Interesting hack. It gets rid of the extra arguments and globals all right, but it also makes the source a bit less obvious to read. Does this play nicely with documentation extraction? AFAICS the docstring must be given on the inner function definition. -- Paul Winkler http://www.slinkp.com

Paul Winkler wrote:

On Fri, May 27, 2005 at 12:08:55PM -0400, Paul Winkler wrote:

...

On Fri, May 27, 2005 at 11:49:03AM -0400, Jim Fulton wrote:

...

BTW, I prefer to code things like this in the following way:

def safe_hasattr(): marker = object()

def safe_hasattr(obj, attr): return getattr(obj, attr, marker) is not marker

return safe_hasattr

safe_hasattr = safe_hasattr()

This way, there aren't any extra arguments or funny globals to confuse matters. (I've seen code that broke because people tried to reuse marker definitions accross modules.

One more thought... as for "funny globals", it seems to me that a "private" global like __marker = [] results in cleaner code than either your approach or the keyword arg hack.

I don't agree. In my approach, there isn't a global to begin with. ...

But of course we don't do it because accessing globals in a method that might be looped over is slow. Which, hopefully, will become a non-issue some day (PEP 267, 268, 280).

Note that in my version above, marker is a local rather than a global and gets looked up at local-variable speed. Jim -- Jim Fulton mailto:jim@zope.com Python Powered! CTO (540) 361-1714 http://www.python.org Zope Corporation http://www.zope.com http://www.zope.org

[Paul Winkler]

...

But of course we don't do it because accessing globals in a method that might be looped over is slow. Which, hopefully, will become a non-issue some day (PEP 267, 268, 280).

[Jim Fulton]

Note that in my version above, marker is a local rather than a global and gets looked up at local-variable speed.

Nope, it's a "cell" reference to an enclosing scope, and gets looked up at cell-dereference speed. In my experience, that's generally a bit slower than accessing a module global. That may have to do with a quirk of MSVC's code generation; unsure. Here's a little program that times it 3 ways: """ def lookup1(arg, _marker=object()): return _marker def lookup2(): _marker = object() def lookup2(arg): return _marker return lookup2 lookup2 = lookup2() _marker = object() def lookup3(arg): return _marker from time import clock as now # s/clock/time/ on Linux from itertools import repeat for f in lookup1, lookup2, lookup3: start = now() for dummy in repeat(None, 1000000): f(dummy) finish = now() print "%-8s %.6g" % (f.__name__, finish - start) """ Under WinXP Python 2.4.1, the global trick (lookup3) is consistently fastest, and the lexical scope trick (lookup2) is consistently slowest; this is typical output across 3 runs: lookup1 0.427597 lookup2 0.450777 lookup3 0.404399 lookup1 0.423195 lookup2 0.471619 lookup3 0.405306 lookup1 0.42321 lookup2 0.448071 lookup3 0.405078 Note that lookup1 has the overhead of installing the default arg into the locals on each call, which is an expense neither lookup1 nor lookup3 endure. measurement-never-agrees-with-anyone<wink>-ly y'rs - tim

[Jim Fulton, on cell dereferencing's (lack of) speed]

This is amazing. Really amazing. I stand corrected.

Well, there's no mystery if you look at LOAD_FAST and LOAD_DEREF in ceval.c; LOAD_DEREF is slower <wink>. The relative speed of approaches changes across Python releases too. For example, module global lookup benefited enormously from a long string of "heroic optimizations" in Python's dict implementation. I consider a difference under 10% to be in the noise, and clawing for one as likely to become a pessimization as to remain an optimization under the next release; e.g., there are some obvious ways to speed LOAD_DEREF (although I don't think anyone at present cares enough to work on that).

On Fri, May 27, 2005 at 01:49:00PM -0400, Tim Peters wrote:

Nope, it's a "cell" reference to an enclosing scope, and gets looked up at cell-dereference speed. In my experience, that's generally a bit slower than accessing a module global. That may have to do with a quirk of MSVC's code generation; unsure.

Wow. If so, gcc apparently shares the same quirk. With python 2.3.5 on my ancient gentoo box, I get this (after substituting time.time for time.clock): lookup1 1.55095 lookup2 1.57738 lookup3 1.4966 This is so suprising to me that I went ahead and tried a couple of variations that I figured *must* be slower: def lookup4(arg): # Doesn't get much simpler than this. return object() class C: # ok this one is pretty silly _marker = object() def lookup5(self, arg): return self._marker lookup5 = C().lookup5 lookup1 1.53485 lookup2 1.57811 lookup3 1.47942 lookup4 2.33403 lookup5 2.0232 measurement-never-agrees-with-anyone-except-when-you-wish-it-didn't-ly y'rs, - Paul -- Paul Winkler http://www.slinkp.com

We're getting "the usual" timing results here: it's a cross-platform, cross-release crapshoot. Aiming at (just) a few percent really is worthless -- modern processors and OSes are too complex to out-think uniformly "in the small", and even if Python variability didn't contribute to the differences. [Tres] ....

But then again, the same is true for pystones:

$ python ~/projects/Zope-CVS/lib/python2.3/test/pystone.py Pystone(1.1) time for 50000 passes = 1.39 This machine benchmarks at 35971.2 pystones/second $ python ~/projects/Zope-CVS/lib/python2.3/test/pystone.py Pystone(1.1) time for 50000 passes = 1.15 This machine benchmarks at 43478.3 pystones/second ...

My favorite in this respect will always be Win98SE. Running pystone after a boot gave a figure almost exactly twice as large (i.e., faster) than running pystone a second, third, ... time. One way to get back the original speed was to reboot. The other way was to write a little Python program that systematically allocated all of RAM, until it died with MemoryErrror. It didn't matter how much RAM you had -- a dozen MB or hundreds, same thing. This was some kind of Satanic OS "reverse caching", I guess <0.5 wink>.

[Tim Peters]

...

def lookup1(arg, _marker=object()): return _marker ... _marker = object() def lookup3(arg): return _marker ... lookup1 0.427597 lookup3 0.404399

[Dieter Maurer]

Do you understand why "lookup3" is faster than "lookup1"?

I had the "impression" that access to the function's local namespace should be faster than any other variable access.

It is, although module-global lookup is often much faster now than Python old-timers "know" it is <wink>: the critical successful path thru the dict lookup code for a dict keyed by interned strings is, in the absence of collisions, _almost_ as lean as an indexed array access now. The primary difference in speed remaining is that the dict lookup endures an extra C-level function call. I explained a relevant difference last time: lookup1 has the additional expense of initializing an additional local variable. That's what a default argument becomes, and nothing happens for free. Let's time this effect "in isolation": """ from itertools import repeat from time import clock as now def with(default=5): pass def without(): pass for dummy in range(3): for f in with, without: start = now() for dummy in repeat(None, 1000000): f() finish = now() print "%-8s %.6g" % (f.__name__, finish - start) """ and typical output: with 0.374435 without 0.338555 with 0.370469 without 0.339791 with 0.372165 without 0.337355 So adding a default argument indeed has a measurable cost (although ~0.03 seconds across a million calls is nothing to fret about!). Note that the difference between lookup1 and lookup3 was less than that on this box, reflecting that the lookup _inside_ lookup1 goes faster than the lookup inside lookup3, partly cancelling lookup1's higher call overhead; the difference in call overhead is greater than the difference in lookup speed, so lookup1 ends up a net loser.

[Tim Peters]

...

OTOH, defining & importing a utility function-- say, safehasattr() --would make it all explicit. That's what ZODB does.

[Paul Winkler]

OK.

(BTW, I just went grepping for this safehasattr() in zope 2.7.6's ZODB and didn't find anything. What's it called?)

No such thing in ZODB 3.2. In 3.3+, it's called "myhasattr", and is imported from ZODB.serialize. It should grow a saner name and move to ZODB.utils instead: def myhasattr(obj, name, _marker=object()): """Make sure we don't mask exceptions like hasattr(). We don't want exceptions other than AttributeError to be masked, since that too often masks other programming errors. Three-argument getattr() doesn't mask those, so we use that to implement our own hasattr() replacement. """ return getattr(obj, name, _marker) is not _marker

There are ~700 calls to hasattr() currently in the Zope core (ouch!). Are there cases where the current use of hasattr() is considered safe?

Almost certainly, although who knows which? I don't. As a general matter, it's hasattr() applied to a _persistent_ object that's especially nasty. Asking whether a persistent object has an attribute can invoke an arbitrary amount of code, including network activity to fetch the object's state, and potentially raising (say) ReadConflictError or assorted socket errors. It's Bad to hide those.

Or since it's "broken by design", should we replace all 700 calls with this hypothetical safe_hasattr()?

Spend an hour analyzing each one <0.9 wink>.

[the rest snipped not because it's uninteresting, but because I don't know enough to comment]

Paul Winkler wrote at 2005-5-27 11:02 -0400:

... def safe_hasattr(obj, attr, acquired=True, _marker=[]): if not acquired: obj = aq_inner(aq_explicit(obj))

This should be "obj = aq_base(obj)". The "aq_explicit(aq_inner(...))" dance is only necessary in untrusted code as "aq_base" is not available there. -- Dieter

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Dieter Maurer wrote:

Tres Seaver wrote at 2005-5-27 08:22 -0400:

...

... As a local patch, this isn't too bad (one could even package it as a do-nothing-after-initialization product). However, no redistributed product code should rely on the presence of a patched 'hasattr', but should use the 3 argument getattr instead.

I think this is *very* bad!

It is quite difficult to get the "hasattr" emulation via "getattr" correct.

If you do not like a modified "__builtin__.hasattr" (which I can understand), then provide at least a "__builtin__.zhasattr" (or similarly named) available in restricted code and strongly recommend in the documentation to use this variant of "hasattr" for any Zope/ZODB object.

As Tim notes, if we aren't replacing the builtin, then we could equally well recommend the following pattern: # in ZODB.utils def safe_hasattr(obj, name, _marker=object()): """Make sure we don't mask exceptions like hasattr(). We don't want exceptions other than AttributeError to be masked, since that too often masks other programming errors. Three-argument getattr() doesn't mask those, so we use that to implement our own hasattr() replacement. """ return getattr(obj, name, _marker) is not _marker & in any module which uses hasattr from ZODB.utils import safe_hasattr .... # was: if hasattr(object, 'attribute'): if safe_hasattr(object, 'attribute'): The spelling, of course, would be up for endless argument. ;) Perhaps we could call it 'persistent_hasattr', and then let folks alias it at import time as they wish (even to 'hasattr', but not in the core!). We would then (as you note) need to ensure that we made it available to restricted code. For that casee, we might need a variant of it which used 'guarded_getattr' (but I'm not sure about that). Tres. - -- =================================================================== Tres Seaver tseaver@palladion.com Palladion Software "Excellence by Design" http://palladion.com -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.5 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCmJ2q+gerLs4ltQ4RAgBHAKC41yH93Xef8Zy0FgEXkauPevp/MACfdOdg d7rWGDewxLvq2+CANiNR6v4= =HQMV -----END PGP SIGNATURE-----