Re: svn commit: rev 7166 - trunk/subversion/libsvn_fs

Greg Stein <gstein@lyra.org> writes:

> That said: I think the per-node pool is a poor idea, though I'm
> hard-pressed to come up with a fix offhand.

Yah. There doesn't seem to be an apr_pool_free()...

> I believe that pool creation is not nearly as cheap as a simple alloc.
> Sander can correct me here, though. I've got a small tweak to improve the
> pool handling a little bit:

[...]

> The net result is that you'll create N pools max, then start reusing
> them.

Yeah, that's a better plan. Will do.

> All that said, I'm not sure that it is relevant to do fine-tuned
> optimizations around this code (i.e. we could avoid some hash lookups,
> some strlen calls, etc). The first-order improvement was to avoid BDB
> calls, which the cache has done. The overall time is probably still driven
> by other BDB calls, and futher optimization here would generally make the
> code a bit harder to read for little gain.

Right -- the real trick here was avoiding 4 or 5 BDB lookups per tree
depth just to find a node. If someone wants to come along later and
change the policy to start counting hit points or something, whatever
-- not on my radar.

> I would suggest, however, you omit the memset() and the zero assignment
> from tree.c::make_root(). You already have an apr_pcalloc() there, so the
> zeroing is completely redundant.

Check!

And thanks for the review.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Sep 24 16:06:30 2003