AW: [PATCH]: Optimize merge_file_trivial() (Was: Fix issue #4128)

Hi, Julian,

> Please use 'TRUE' and 'FALSE' for Boolean values in the code and in the doc strings, not zero and non-zero.

I did just copy this from the 2-file functions that existed. I'll update the patch accordingly.

> Please replace 'rsp' with 'and' or 'or' in the doc strings. See <http://www.transblawg.eu/index.php?/archives/870-Resp.-and-other-non-existent-English-wordsNicht-existente-englische-Woerter.html> :-)

> Please don't initialize variables that are going to be unconditionally initialized later: variables 'file1_h', 'file2_h', 'file3_h' in contents_three_identical_p().

I'll fix both issues, too.

> Please make all the indentation consistent, and put operators on the beginning of each continuation line indented to just inside the relevant opening parenthesis, so instead of this:

Is there some reformatter script or (even better) visual studio plugin that implements all the formatting rules?

Best regards

Markus Schaber

--
We software Automation.
3S-Smart Software Solutions GmbH
Markus Schaber | Developer
Memminger Str. 151 | 87439 Kempten | Germany | Tel. +49-831-54031-0 | Fax +49-831-54031-50
Email: m.schaber@3s-software.com | Web: http://www.3s-software.com
CoDeSys internet forum: http://forum.3s-software.com
Download CoDeSys sample projects: http://www.3s-software.com/index.shtml?sample_projects
Managing Directors: Dipl.Inf. Dieter Hess, Dipl.Inf. Manfred Werner | Trade register: Kempten HRB 6186 | Tax ID No.: DE 167014915
________________________________________
Von: Julian Foad [julianfoad_at_btopenworld.com]
Gesendet: Mittwoch, 13. Juni 2012 14:22
Bis: Markus Schaber
Cc: dev_at_subversion.apache.org
Betreff: Re: [PATCH]: Optimize merge_file_trivial() (Was: Fix issue #4128)
Hi Markus.  Only some stylistic comments from me; the basic idea looks fine.
Please use 'TRUE' and 'FALSE' for Boolean values in the code and in the doc strings, not zero and non-zero.
Please replace 'rsp' with 'and' or 'or' in the doc strings.  See <http://www.transblawg.eu/index.php?/archives/870-Resp.-and-other-non-existent-English-wordsNicht-existente-englische-Woerter.html>  :-)
Please don't initialize variables that are going to be unconditionally initialized later: variables 'file1_h', 'file2_h', 'file3_h' in contents_three_identical_p().
Please make all the indentation consistent, and put operators on the beginning of each continuation line indented to just inside the relevant opening parenthesis, so instead of this:
+contents_three_identical_p(svn_boolean_t *identical_p12,
+                     svn_boolean_t *identical_p23,
[...]
+      if (*identical_p12 && !(eof1 && eof2) &&
+            ((eof1 != eof2) ||
+            (bytes_read1 != bytes_read2) ||
+            memcmp(buf1, buf2, bytes_read1)))
should be
+contents_three_identical_p(svn_boolean_t *identical_p12,
+                           svn_boolean_t *identical_p23,
[...]
+      if (*identical_p12 && !(eof1 && eof2)
+          && ((eof1 != eof2)
+              || (bytes_read1 != bytes_read2)
+              || memcmp(buf1, buf2, bytes_read1)))
Thanks.
- Julian
----- Original Message -----
> From: Markus Schaber <m.schaber_at_3s-software.com>
> To: "dev_at_subversion.apache.org" <dev_at_subversion.apache.org>
> Cc:
> Sent: Wednesday, 13 June 2012, 12:45
> Subject: [PATCH]: Optimize merge_file_trivial() (Was: Fix issue #4128)
>
> Hi, all,
>
> Von: Philip Martin [philip.martin_at_wandisco.com]
>>>  [Context: merge_file_trivial() in merge.c]
>>
>>  I wonder if we could reduce the file IO by comparing all three files in
>>  one operation instead of two operations on pairs of files?
>
> In follow-up to my last patch fixing issue #4128, the following patch optimizes
> merge_file_trivial(). It avoids reading the files twice by using a new
> comparison function which compares 3 files at once.
>
> Open issues (AFAICS):
> - The naming of the functions. (I did use the word three instead of the cipher 3
> to avoid confusion with revisioned APIs.)
> - The order of parameters. I'm not sure whether 12,23,13 is more natural, or
> 12,13,23.
>
> Summary of test results:
>   1618 tests PASSED
>   85 tests SKIPPED
>   39 tests XFAILED (1 WORK-IN-PROGRESS)
>
> [[
> Optimize merge_file_trivial() by avoiding to read the files twice by using a
> new comparison function which compares 3 files at once.
>
> * subversion/include/svn_io.h
>   (svn_io_filesizes_three_different_p): Add new declaration
>   (svn_io_files_contents_three_same_p): Add new declaration
>
> * subversion/libsvn_subr/io.c
>   (svn_io_filesizes_three_different_p): Add new function in analogy to
> svn_io_filesizes_different_p().
>   (contents_three_identical_p): Add new function in analogy to
> contents_identical_p().
>   (svn_io_files_contents_three_same_p): Add new function in analogy to
> svn_io_files_contents_same_p.
>
> * subversion/libsvn_wc/merge.c
>   (merge_file_trivial): Use the new three-file comparison functions to avoid
> reading files twice.
>
> Patch by: Markus Schaber <m.schaber_at_3s-software.com>
> ]]
>
>
> Thank you,
> Markus Schaber
> --
>
> We software Automation.
>
> 3S-Smart Software Solutions GmbH
> Markus Schaber | Developer
> Memminger Str. 151 | 87439 Kempten | Germany | Tel. +49-831-54031-0 | Fax
> +49-831-54031-50
>
> Email: m.schaber@3s-software.com | Web: http://www.3s-software.com
> CoDeSys internet forum: http://forum.3s-software.com
> Download CoDeSys sample projects:
> http://www.3s-software.com/index.shtml?sample_projects
>
> Managing Directors: Dipl.Inf. Dieter Hess, Dipl.Inf. Manfred Werner | Trade
> register: Kempten HRB 6186 | Tax ID No.: DE 167014915
>