[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: r1397773 - rep-sharing in a txn - subversion/libsvn_fs_fs/fs_fs.c

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Mon, 19 Nov 2012 15:16:44 +0000 (GMT)

Stefan Fuhrmann wrote:

On Thu, Nov 15, 2012 at 6:16 PM, Julian Foad <julianfoad_at_btopenworld.com> wrote:
>>stefan2_at_apache.org wrote:
>>> URL: http://svn.apache.org/viewvc?rev=1397773&view=rev
>>> Log:
>>> Due to public request: apply rep-sharing to equal data reps within
>>> the same transaction.
>>>
>>> The idea is simple. When writing a noderev to the txn folder,
>>> write another file named by the rep's SHA1 and store the rep
>>> struct in there. Lookup is then straight-forward.
>>
>> Please could you update the FSFS structure document.  I assume it
>> says what files are stored in the txn dir; if it doesn't it should.
>
> Oops - hadn't thought about that! Done in r1411202.
> All other changes in r1411209.

Thanks, Stefan.  Looks good.
>>> +  /* in our txn, is there a rep file named with the wanted SHA1?

>>> +     If so, read it and use that rep. */
>>> +  SVN_ERR(svn_io_check_path(file_name, &kind, pool));
>>
>> Instead of stat'ing the file to decide whether to open it, it's
>> more efficient and more 'robust' to just try to open it and then
>> handle the potential failure, isn't it?
>
> Access is serialized (see below). Checking for existence
> should be more efficient because actual matches are
> relatively rare and constructing an error object can be
> expensive (NLS). svn_stringbuf_from_file2 is also a very
> convenient function to use.
I wonder if we could change our generic error handling scheme to defer NLS look-up until the time when we print the error.  That would eliminate that overhead in cases where we handle and clear an error without printing it.  Something to think about for the future.

[...]

> Thanks for the review! The code should be multi-thread safe.

Thanks.

- Julian
Received on 2012-11-19 16:17:21 CET

This is an archived mail posted to the Subversion Dev mailing list.