Why does tp_clear have a return value? All the code I've seen returns 0, but the only place that clear is called doesn't inspect its return value. Jeremy
On Wed, Apr 09, 2003 at 03:33:47PM -0400, Jeremy Hylton wrote:
Why does tp_clear have a return value? All the code I've seen returns 0, but the only place that clear is called doesn't inspect its return value.
I guess I would have to say overdesign. I was thinking that tp_clear and tp_traverse could somehow be used by things other than the GC. In retrospect that doesn't seem likely or even possible. The GC has pretty specific requirements. In retrospect, I think both tp_traverse and tp_clear should have returned "void". That would have made implementing those methods easier. Testing for errors in tp_traverse methods is silly since nothing returns an error, and, even if it did, the GC couldn't handle it. :-( How do we sort this out? I suppose one step would be to document that the return values of tp_traverse and tp_clear are ignored. If we agree on that, I volunteer to go through the code and remove the useless tests for errors in the tp_traverse methods. Neil
On Wed, Apr 09, 2003 at 03:33:47PM -0400, Jeremy Hylton wrote:
Why does tp_clear have a return value? All the code I've seen returns 0, but the only place that clear is called doesn't inspect its return value.
[In response, Neil admitted]
I guess I would have to say overdesign. I was thinking that tp_clear and tp_traverse could somehow be used by things other than the GC. In retrospect that doesn't seem likely or even possible. The GC has pretty specific requirements.
In retrospect, I think both tp_traverse and tp_clear should have returned "void". That would have made implementing those methods easier. Testing for errors in tp_traverse methods is silly since nothing returns an error, and, even if it did, the GC couldn't handle it.
:-(
How do we sort this out? I suppose one step would be to document that the return values of tp_traverse and tp_clear are ignored. If we agree on that, I volunteer to go through the code and remove the useless tests for errors in the tp_traverse methods.
That's a good first step. Unfortunately changing the declaration to void will break 3rd party extensions so that will be too painful. --Guido van Rossum (home page: http://www.python.org/~guido/)
Neil Schemenauer wrote:
I guess I would have to say overdesign. I was thinking that tp_clear and tp_traverse could somehow be used by things other than the GC. In retrospect that doesn't seem likely or even possible. The GC has pretty specific requirements.
In retrospect, I think both tp_traverse and tp_clear should have returned "void".
While this is true for tp_clear, tp_traverse is actually more general. gc.get_referrers uses tp_traverse, for something other than collection.
That would have made implementing those methods easier. Testing for errors in tp_traverse methods is silly since nothing returns an error, and, even if it did, the GC couldn't handle it.
Again, gc.get_referrers "uses" this feature. If extending the list fails, traversal is aborted. Whether this is useful is questionable, as the entire notion of "out of memory exception handling" is questionable. Regards, Martin
[Neil Schemenauer]
I was thinking that tp_clear and tp_traverse could somehow be used by things other than the GC. In retrospect that doesn't seem likely or even possible. The GC has pretty specific requirements. In retrospect, I think both tp_traverse and tp_clear should have returned "void".
[Martin v. Lowis]
While this is true for tp_clear, tp_traverse is actually more general. gc.get_referrers uses tp_traverse, for something other than collection.
That would have made implementing those methods easier. Testing for errors in tp_traverse methods is silly since nothing returns an error, and, even if it did, the GC couldn't handle it.
Again, gc.get_referrers "uses" this feature. If extending the list fails, traversal is aborted. Whether this is useful is questionable, as the entire notion of "out of memory exception handling" is questionable.
The brand new gc.get_referents uses the return value of tp_traverse to abort on out-of-memory, but gc.get_referrers uses it for a different purpose (its traversal function returns true if the visited object is in the tuple of objects passed in, else returns false). The internal gc.get_referrers_for is what aborts on out-of-memory in the get_referrers subsystem. tp_traverse is fine as-is. The return value of tp_clear does indeed appear without plausible use.
If we agree that, I volunteer to go through the code and remove the useless tests for errors in the tp_traverse methods.
That would make get_referents press on after memory is exhausted. It would also change the semantics of get_referrers, in a subtle way (if object A has 25 references to object B, gc.get_referrers(B) contains only 1 instance of A today, but would contain 25 instances of A if tp_traverse methods ignored visit() return values). truth-isn't-necessarily-an-error-ly y'rs - tim
"Martin v. L?wis" wrote:
Neil Schemenauer wrote:
In retrospect, I think both tp_traverse and tp_clear should have returned "void".
While this is true for tp_clear, tp_traverse is actually more general. gc.get_referrers uses tp_traverse, for something other than collection.
Could the visit procedure keep track of errors? Something like: struct result { int error; /* true if an error occured while traversing */ /* other results */ } static void myvisit(PyObject* obj, struct result *r) { if (!r->error) { <do stuff, set r->error of error occurs> } }
Neil Schemenauer wrote:
Could the visit procedure keep track of errors?
No. For get_referrers (as Tim explains), it might be acceptable but less efficient (since traversal should stop when a the object is found to be a referrer). For get_referents, an error in the callback should really abort traversal as the system just went out of memory. Regards, Martin
[Neil Schemenauer]
Could the visit procedure keep track of errors?
[Martin v. Löwis]
No. For get_referrers (as Tim explains), it might be acceptable but less efficient (since traversal should stop when a the object is found to be a referrer). For get_referents, an error in the callback should really abort traversal as the system just went out of memory.
Still, I expect both could be handled by setjmp in the gc module get_ref* driver functions and longjmp (as needed) in the gc module visitor functions. IOW, the tp_traverse slot functions don't really need to cooperate, or even know anything about "early returns". Why this may be more than just idly interesting: the tp_traverse functions are called a lot by gc. The get_ref* functions are never called except when explicitly asked for, and their speed just doesn't matter. Burdening them with funky control flow would be a real win if eliminating almost-always-useless test/branch constructs in often-called tp_traverse slots sped the latter.
"Tim Peters" <tim_one@email.msn.com> writes:
Still, I expect both could be handled by setjmp in the gc module get_ref* driver functions and longjmp (as needed) in the gc module visitor functions. IOW, the tp_traverse slot functions don't really need to cooperate, or even know anything about "early returns".
That would require that tp_traverse does not modify any refcount while iterating, right? It seems unpythonish to use setjmp/longjmp for exceptions. Regards, Martin
[Tim]
Still, I expect both could be handled by setjmp in the gc module get_ref* driver functions and longjmp (as needed) in the gc module visitor functions. IOW, the tp_traverse slot functions don't really need to cooperate, or even know anything about "early returns".
[martin@v.loewis.de]
That would require that tp_traverse does not modify any refcount while iterating, right?
Or do anything else that relies on calls to visit() returning. I've looked at every traverse slot in the core, and there's no problem with those. I don't think that's an accident -- the only purpose of an object's tp_traverse is to invoke the visit callback on the non-NULL PyObject* pointers the object has. So, e.g., there isn't an incref or decref in any of 'em now; at worst there's an int loop counter.
It seems unpythonish to use setjmp/longjmp for exceptions.
I'm not suggesting adding setjmp/longjmp to the Python language <0.9 wink>. I'm suggesting using them for two specific and obscure gc module callbacks that aren't normally used (*most* of the gc module callbacks wouldn't use setjmp/longjmp); in return, mounds of frequently executed code like static int func_traverse(PyFunctionObject *f, visitproc visit, void *arg) { int err; if (f->func_code) { err = visit(f->func_code, arg); if (err) return err; } if (f->func_globals) { err = visit(f->func_globals, arg); if (err) return err; } if (f->func_module) { err = visit(f->func_module, arg); if (err) return err; } if (f->func_defaults) { err = visit(f->func_defaults, arg); if (err) return err; } if (f->func_doc) { err = visit(f->func_doc, arg); if (err) return err; } ... return 0; } could become the simpler and faster static int func_traverse(PyFunctionObject *f, visitproc visit, void *arg) { int err; if (f->func_code) visit(f->func_code, arg); if (f->func_globals) visit(f->func_globals, arg); if (f->func_module) visit(f->func_module, arg); if (f->func_defaults) visit(f->func_defaults, arg); if (f->func_doc) visit(f->func_doc, arg); ... return 0; } (I kept the final return 0 so that the signature wouldn't change.)
On Thu, 2003-04-10 at 14:09, Tim Peters wrote:
I'm not suggesting adding setjmp/longjmp to the Python language <0.9 wink>. I'm suggesting using them for two specific and obscure gc module callbacks that aren't normally used (*most* of the gc module callbacks wouldn't use setjmp/longjmp); in return, mounds of frequently executed code like
...
could become the simpler and faster
... Sure sounds good to me. If traverse worked this way, the traverse and clear slots and a part of the dealloc slot become almost identical. The take all PyObject * members in the struct and perform some action on them if they are non-NULL. dealloc performs a DECREF. clear performs a DECREF + assign NULL. traverse calls visit. It sure makes it easy to verify that each is implemented correctly. It would be cool if there were a way to automate some of the boilerplate. Jeremy
If traverse worked this way, the traverse and clear slots and a part of the dealloc slot become almost identical. ... It would be cool if there were a way to automate some of the boilerplate.
There is... use Pyrex. :-) Greg Ewing, Computer Science Dept, +--------------------------------------+ University of Canterbury, | A citizen of NewZealandCorp, a | Christchurch, New Zealand | wholly-owned subsidiary of USA Inc. | greg@cosc.canterbury.ac.nz +--------------------------------------+
On Thu, 2003-04-10 at 16:26, Martin v. Löwis wrote:
Tim Peters <tim.one@comcast.net> writes:
could become the simpler and faster
How much faster, and for what example? Beautiful is better than ugly.
Doesn't "beautiful is better than ugly" mean that a little ugliness in the gcmodule allows all the client code to be beautiful? Jeremy
[Tim]
could become the simpler and faster
[martin@v.loewis.de]
How much faster,
Won't know until it's tried.
and for what example?
Code that spends signficant time in tp_traverse, presumably.
Beautiful is better than ugly.
Whish is another reason <wink> it would be nice to get rid of the endlessly repeated masses of ugly if (err) return err; incantations out of the many tp_traverse slots, in return for putting a little bit of setjmp/longjmp ugliness in exactly four functions hiding in a single module.
Tim Peters wrote:
[martin@v.loewis.de]
Beautiful is better than ugly.
Whish is another reason <wink> it would be nice to get rid of the endlessly repeated masses of ugly
if (err) return err;
incantations out of the many tp_traverse slots, in return for putting a little bit of setjmp/longjmp ugliness in exactly four functions hiding in a single module.
I agree that concentrating the ugliness is good. However, how portable is setjmp/longjmp? The manual page I have says C99. Can we rely on it being available? If not, could we just disable the gcmodule functions that depend on it? Neil
[Neil Schemenauer]
I agree that concentrating the ugliness is good. However, how portable is setjmp/longjmp? The manual page I have says C99.
It's also C89, i.e. "ANSI C".
Can we rely on it being available?
I think so. Note that we have three modules that use them now, although they're not compiled everywhere (readline, pcre, fpectl).
If not, could we just disable the gcmodule functions that depend on it?
Jeremy and I have spent a lot of time tracking down leaks (in Python and in Zope) recently, and get_refer{rers, ents} have been invaluable. If we found a platform where {set,long}jmp didn't work, I'd be OK with disabling those two gc functions on that platform. Those functions aren't needed for normal gc operation, and it's not any platform I'm going to be using anyway <wink>.
it would be nice to get rid of the endlessly repeated ... ugly incantations out of the many tp_traverse slots, in return for putting a little bit of setjmp/longjmp ugliness in exactly four functions hiding in a single module.
I'd be pretty nervous about having any longjmps anywhere near anything Python. If you do this, you'll have to make it very clear that tp_traverse implementations MUST NOT alter any Python ref counts, or rely in any other way on running to completion. Greg Ewing, Computer Science Dept, +--------------------------------------+ University of Canterbury, | A citizen of NewZealandCorp, a | Christchurch, New Zealand | wholly-owned subsidiary of USA Inc. | greg@cosc.canterbury.ac.nz +--------------------------------------+
[Greg Ewing]
I'd be pretty nervous about having any longjmps anywhere near anything Python.
Why?
If you do this, you'll have to make it very clear that tp_traverse implementations MUST NOT alter any Python ref counts, or rely in any other way on running to completion.
That's so. For reasons explained earlier, it would be quite surprising to see a tp_traverse function play with anything's refcount (their purpose is to pass an object's PyObject* pointers on to the callback argument, and that's all; manipulating refcounts during this wouldn't make sense).
it would be quite surprising to see a tp_traverse function play with anything's refcount (their purpose is to pass an object's PyObject* pointers on to the callback argument, and that's all
A thought -- maybe tp_visit and tp_clear could be unified by having a tp_visit that passed pointers to pointers to objects to the callback? Greg Ewing, Computer Science Dept, +--------------------------------------+ University of Canterbury, | A citizen of NewZealandCorp, a | Christchurch, New Zealand | wholly-owned subsidiary of USA Inc. | greg@cosc.canterbury.ac.nz +--------------------------------------+
[Greg Ewing]
A thought -- maybe tp_visit and tp_clear could be unified by having a tp_visit that passed pointers to pointers to objects to the callback?
I think Jeremy suggested something like that earlier today. I don't think it would fly now. tuples are the simplest example of a gc container object whose tp_clear and tp_traverse slot functions do radically different things (the tuple tp_clear is NULL!); type objects may be the most complex example (see the long comment block in typeobject.c's type_clear for an explanation of why only tp_mro is-- or needs to be --cleared). In general, tp_traverse needs to reveal every PyObject* that may be part of a cycle, but tp_clear only needs to nuke the subset of those necessary to guarantee that all cycles will be broken. OTOH, I suspect Guido thought too hard about this. Like the tp_clear comment: tp_dict: It is a dict, so the collector will call its tp_clear. If type_clear decrefed tp_dict, and the refcount fell to 0 thereby, the usual refcount mechanism would nuke the dict on its own, and the collector would *not* in fact call the dict's tp_clear slot (the dict object would get unlinked from the gc list it was in, and the collector would never see the dict again). So I'm unclear on what we're trying to optimize when a tp_clear nukes less than the corresponding tp_traverse visits. I suppose "code space" is one decent answer to that.
Tim Peters <tim.one@comcast.net> writes:
So I'm unclear on what we're trying to optimize when a tp_clear nukes less than the corresponding tp_traverse visits. I suppose "code space" is one decent answer to that.
In the case of type objects, it's not a matter of optimization but of correctness. If you were clearing all slots of a type object, you'd lose state that is still needed later on; see the comment for typeobject.c:2.150. Regards, Martin
So I'm unclear on what we're trying to optimize when a tp_clear nukes less than the corresponding tp_traverse visits. I suppose "code space" is one decent answer to that.
Yes. Though the type object example shows there are other differences (thanks Martin). --Guido van Rossum (home page: http://www.python.org/~guido/)
I've looked at every traverse slot in the core, and there's no problem with those. I don't think that's an accident -- the only purpose of an object's tp_traverse is to invoke the visit callback on the non-NULL PyObject* pointers the object has. So, e.g., there isn't an incref or decref in any of 'em now;
But what about the *visit function*? You need to take account of what it might do as well. And if it's ever used for something beside GC, it could do anything. Greg Ewing, Computer Science Dept, +--------------------------------------+ University of Canterbury, | A citizen of NewZealandCorp, a | Christchurch, New Zealand | wholly-owned subsidiary of USA Inc. | greg@cosc.canterbury.ac.nz +--------------------------------------+
[Greg Ewing]
But what about the *visit function*? You need to take account of what it might do as well. And if it's ever used for something beside GC, it could do anything.
I don't see the relevance. The visit functions are where the longjmps would go, if a visit function felt like using one. Two visit functions in gcmodule.c would use them, the other visit functions in gcmodule.c would not. I don't know of any visit functions not in gcmodule.c (where they all have static scope), nor do I expect to see any outside of gcmodule.c -- visit functions are Python internals. tp_clear and tp_traverse functions must be supplied by extension authors who want their types to play with the gc system, but extension authors are never required (or even asked) to write a visit function.
participants (9)
-
"Martin v. Löwis" -
Greg Ewing -
Guido van Rossum -
Jeremy Hylton -
martin＠v.loewis.de
-
Michael Hudson -
Neil Schemenauer -
Tim Peters -
Tim Peters