Hi
On Wed, Oct 29, 2008 at 9:54 PM, Joe Swatosh <joe.swatosh_at_gmail.com> wrote:
> Howdy Hyrum, kou,
>
> On Mon, Oct 27, 2008 at 8:08 AM, Hyrum K. Wright
> <hyrum_wright_at_mail.utexas.edu> wrote:
>> Joe Swatosh wrote:
>>> Hi Hyrum,
>>>
>>> On Sat, Oct 18, 2008 at 6:12 AM, <hwright_at_tigris.org> wrote:
>>>> Author: hwright
>>>> Date: Sat Oct 18 06:11:59 2008
>>>> New Revision: 33730
>>>>
>>>> Log:
>>>> Merge the fs-rep-sharing branch to trunk.
>>>>
>>>> This gives us significant space savings on both bdb and fsfs. See the
>>>> branch for individual log messages.
>>>>
>>>
>>> Obviously, this commit broke the windows build, but when it was fixed
>>> in r33743 none of the Ruby bindings tests would even run anymore. I
>>> instrumented the calls to sqlite3_open_v2() and sqlite3_close() and
>>> many of the calls from the bindings into svn. Below is the output
>>> along with some comments by me marked by #'s. I hope I'm talking
>>> sense. This was run against trunk at r33894.
>>>
>>>
snip
>>>
>>> I'm just a bystander watching the development of svn proper, but I wonder
>>> if there is a better pool to put the clean up of the cache opened by the
>>> commit call above? It seems like ideally, it should be released before
>>> the invocation of commit completes (like checkout).
>>
>> The rep-sharing sqlite database currently closes itself when the pool provided
>> to svn_fs_new() or svn_fs_open() is destroyed. I suppose that if a caller
>> doesn't destroy the pool, but instead uses it on a subsequent call to
>> svn_fs_open(), then we'll have two open handles to the sqlite database. That in
>> and of itself shouldn't be too much of a problem, but I'm wondering if the
>> cleanup function we register with APR is getting confused.
>>
>> Thoughts?
>>
>
> Yup, the sqlite database is closed, but for whatever reason
> svn_client_commit4 and svn_client_list2 (and others) are just passing
> along the pool that was passed to them. I don't think the cleanup
> function is confused, I just think the lifetime of the pool we are
> passing in is too long. This might not even be problem on non-Windows
> OSes that can delete open files.
>
> The apparent reason that svn_client_checkout3 was working for me was
> that it calls svn_client__checkout_internal, which creates a temporary
> "session_pool" that is passed to svn_ra_open3 and so the sqlite
> database is closed before returning to Ruby.
>
> I started down the path of creating a patch for the svn_client library
> mimicking that pattern of creating session_pools, but when I got to
> svn_client_open_ra_session, I started doubting the approach. It seems
> to me that if we are returning an RA session object that perhaps the
Trying to make this a little more concrete, I've attached the patch in
progress. The more I think about it the less I like it. It seems
like there may be synchronization issues with having different
functions accessing the cache with different file handles.
--
Joe
> handle to the cache ought to stick around for the lifetime of the
> session object. Or if that isn't correct maybe the bindings should
> create a pool for the duration? I looked at this a little, but I'm
> not sure how to accomplish that. Or maybe I should just continue
> creating session pools and post the patch?
>
> More questions than answers, I'm afraid.
> --
> Joe
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-11-03 06:48:41 CET