On 4/6/07, Malcolm Rowe <malcolm-svn-dev@farside.org.uk> wrote:
> On Thu, Apr 05, 2007 at 06:33:26PM -0600, D.J. Heap wrote:
> > Could someone review and test this patch on Linux? I think this is
> > the simplest way to solve the problem, but I'm not positive.
> >
>
> I've not reviewed this thoroughly, but two observations:
>
> - there's no need to rename the functions to init2() - they're private
> and only called via a private vtable, so there are no compatibility
> concerns.
Hmm, I got the impression they were DSO functions on Linux and it
looked like they were called before we did real version checking
(since they read the version info).
Are you saying it's not possible (or not enough of a problem to worry
about) for old versions of the fs modules to be called (and possibly
crash instead of reporting version mismatch) with the new parameter?
That would make things a bit easier.
>
> - if you're going to pass 'the' common pool (I assume the same one
> that's passed to the serialised_init function?) down to the module
> init function, could you call it 'common_pool' in the function
> arguments and note that it's guaranteed to be the same pool in the
> doc-comments?
Sure, good idea.
>
>
> Finally, one thing that really bugs me about our current scheme is that
> there's no way to gain access to the common pool at fs open or create
> time, which means that there's a fair amount of caching-type things that
> we just can't do.
>
> [Recall that the series of calls through the vtable is e.g. "open",
> "serialised_init(common_pool)", where only the latter is guaranteed to
> be serialised with other calls (to serialised_init).]
>
> I'd much prefer it if we could change the vtable to eliminate the
> separate serialised_init call, and just serialise the open/create calls
> (against other open/create calls). (I don't know if there was some
> reason we didn't do that initially - perhaps I'm missing something?).
> Is this something that fits with what you're trying to fix as well?
The only reason I can think of is some compatibility concern, or maybe
a performance concen about serializing create/open? I can't see why
it would be a concern, but I'm not deadly on this code. Maybe Garrett
or Brane can comment on the pros/cons they see in doing that?
If it's a good idea, then we could eliminate serialized_init, add a
common_pool parameter (and serialize the calls) to create/open? Does
it seem reasonable to keep common_pool as a parameter to the init
functions (and leave the bdb cache init there) as well as add it to
the create/open?
Thanks for the help and review!
DJ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Apr 6 17:02:24 2007