On Thu, Sep 20, 2001 at 07:54:22PM -0700, Ryan Bloom wrote: > On Thursday 20 September 2001 05:48 pm, Greg Stein wrote: ... > > Calling pop_cleanup() on every iteration is a bit much. Consider the > > following patch: > > Why is it a bit much? I just took a quick look at it, it is an if, and three > assignments.
Heh. It is a bit much when you consider you consider a thread in October 1999 discussing using NULL rather than ap_null_cleanup: http://www.humanfactor.com/cgi-bin/cgi-delegate/apache-ML/nh/1999/Oct/0189.html In that case, an "if" was considered too much :-) > I would assume that any compiler worth it's salt would inline this function > as well. Very bad assumption. But this isn't the place to discuss compilers. > This patch also keeps the LIFO behavior, which is important, because it means > that it is much less likely that an item allocated out of the pool when the > cleanup > was registered will no longer be there when the cleanup is run. I am telling you that dependency upon items in the pool is bogus. You cannot do that. Thus, LIFO means diddly squat. Consider a case where A inserts its cleanup, expecting to have B present. At some point between then and the pool cleanup, B goes and removes its cleanup, then inserts some other cleanup. This new one, B', will now run *before* A's cleanup. Gee. Guess what? Your oh-so-needed LIFO behavior has been screwed. B is *not* present for A to use. The *only* guarantees you have are between parent and child pools. Within a pool, you've got nothing. > > while ((c = p->cleanups) != NULL) { > > p->cleanups = NULL; > > run_cleanups(c); > > } > > Which function is this in? I have looked, but the only place that I can find > to > put this code would be in apr_pool_clear, around the run_cleanups code. Yes, it would go in apr_pool_clear. And a similar one for the child cleanup case. > > Basically, the above code processes the cleanups in batches. Everything > > that was initially registered is processed, then everything registerd > > during the first cleanup round, etc. > > This makes it more more likely that a variable in the same pool that was > available > when the cleanup was registered would not be available when your cleanup > ran. I would really want to see a performance analysis before we broke that > behavior. Broke what behavior? Registering a cleanup in a cleanup doesn't work now. The new behavior is, "if you register it, I'll run it after I'm done running these other cleanups." Stop. Take a breath. Think about it. LIFO doesn't matter. -g -- Greg Stein, http://www.lyra.org/
