Julian Foad wrote:
> On Sun, 2010-05-02, Stefan Fuhrmann wrote:
>
>> Bert Huijben wrote:
>>
>>>> Stefan Fuhrmann wrote:
>>>>
>>>> * subversion/include/svn_delta.h
>>>> (enum svn_delta_action): ensure that these definitions
>>>> will always have these values, even if new definitions
>>>> were added
>>>>
>>> This part is not necessary. These values are part of our public API
>>> compatibility promise and that makes the values static for 1.X.
>>>
>>> The C compiler defines the same values... and if it didn't you are not
>>> allowed to change the values (for the same binary compatibility reasons).
>>>
>>>
>> Correct. I just wanted these values to become more visible
>> to anyone who might want to extend the enumeration.
>>
>> But I'm fine with removing that part. If the encoding should
>> be changed accidentally, virtually any repository read access
>> will fail with a checksum error. So, it would not linger there
>> unnoticed. So, please remove that part if you apply the patch.
>>
>
> I compromised here by just adding a note that the svndiff implementation
> relies on these values.
>
>
>>>> (copy_source_ops): dito; optimize a common case
>>>>
>>>> * subversion/libsvn_delta/svndiff.c
>>>> (decode_file_offset, decode_size): use slightly more
>>>> efficient formulations
>>>> (decode_instruction): directly map action codes -
>>>> made possible by defining the enum values
>>>>
>>> The enum values were already defined so that doesn't change anything (see
>>> above: they can't change anyway).
>>>
>>>
>> O.k. omit the "- made possible ..." part.
>>
>
> Done.
>
>
>>> Optimizing a % operator for values of 1 and 2 looks like code obfuscation to
>>> me, unless you can show me some numbers that it actually makes a difference.
>>> I would assume that the processor optimizes that case itself.
>>>
>>>
>> The performance advantage is very small anyway - no
>> reason for me to put any more effort in it. Just leave it
>> out from the commit.
>>
>
> OK, left out.
>
>
> Stefan, thanks for persisting and answering all our questions.
>
> This looks good now. I'm committing it.
>
> Committed revision 941243.
>
Thanks for committing! I just got the last patch set out
(zlib) and will start detailed performance measurements.
Hopefully, all my other patches will then get committed
as well ;)
Currently, I get 2.6s (real) for an svn export -q of
svn://localhost/apache/subversion/trunk and 1.6s (real)
if I disable compression for the data transfer.
-- Stefan^2.
Received on 2010-05-09 23:26:24 CEST