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.
>
>> (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.
> 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.
-- Stefan^2.
Received on 2010-05-02 14:45:05 CEST