You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ph...@apache.org on 2012/07/02 16:34:14 UTC
svn commit: r1356261 - in /subversion/trunk: build.conf
subversion/include/svn_io.h subversion/libsvn_subr/io.c
subversion/libsvn_wc/merge.c subversion/tests/libsvn_subr/
subversion/tests/libsvn_subr/io-test.c
Author: philip
Date: Mon Jul 2 14:34:11 2012
New Revision: 1356261
URL: http://svn.apache.org/viewvc?rev=1356261&view=rev
Log:
Optimize merge_file_trivial() by avoiding to read the files twice by
using a new comparison function which compares 3 files at once. Also
add C tests for the new and existing file comparison functions in
libsvn_subr/io.c
* 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.
* build.conf
(io-test): Add the new io-test.c file to the build configuration.
* subversion/tests/libsvn_subr:
Add the executable and temporary test file folder to svn:ignore.
* subversion/tests/libsvn_subr/io-test.c
(create_test_file): Helper function to create a test data file.
(create_comparison_candidates): Helper function to create the full
set of test data files.
(test_two_file_size_comparison): Test function for
svn_io_filesizes_different_p.
(test_two_file_content_comparison): Test function for
svn_io_files_contents_same_p.
(test_three_file_size_comparison): Test function for
test_three_file_size_comparison.
(test_three_file_content_comparison): Test function for
svn_io_files_contents_three_same_p.
Patch by: Markus Schaber <m....@3s-software.com>
Added:
subversion/trunk/subversion/tests/libsvn_subr/io-test.c (with props)
Modified:
subversion/trunk/build.conf
subversion/trunk/subversion/include/svn_io.h
subversion/trunk/subversion/libsvn_subr/io.c
subversion/trunk/subversion/libsvn_wc/merge.c
subversion/trunk/subversion/tests/libsvn_subr/ (props changed)
Modified: subversion/trunk/build.conf
URL: http://svn.apache.org/viewvc/subversion/trunk/build.conf?rev=1356261&r1=1356260&r2=1356261&view=diff
==============================================================================
--- subversion/trunk/build.conf (original)
+++ subversion/trunk/build.conf Mon Jul 2 14:34:11 2012
@@ -757,6 +757,14 @@ sources = hashdump-test.c
install = test
libs = libsvn_test libsvn_subr apriconv apr
+[io-test]
+description = Test I/O Operations
+type = exe
+path = subversion/tests/libsvn_subr
+sources = io-test.c
+install = test
+libs = libsvn_test libsvn_subr apriconv apr
+
[opt-test]
description = Test options library
type = exe
Modified: subversion/trunk/subversion/include/svn_io.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_io.h?rev=1356261&r1=1356260&r2=1356261&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_io.h (original)
+++ subversion/trunk/subversion/include/svn_io.h Mon Jul 2 14:34:11 2012
@@ -607,6 +607,25 @@ svn_io_filesizes_different_p(svn_boolean
const char *file2,
apr_pool_t *pool);
+/** Set @a *different_p12 to non-zero if @a file1 and @a file2 have different
+ * sizes, else set to zero. Do the similar for @a *different_p23 with
+ * @a file2 and @a file3, and @a *different_p13 for @a file1 and @a file3.
+ * All three of @a file1, @a file2 and @a file3 are utf8-encoded.
+ *
+ * Setting @a *different_p12 to zero does not mean the files definitely
+ * have the same size, it merely means that the sizes are not
+ * definitely different. That is, if the size of one or both files
+ * cannot be determined, then the sizes are not known to be different,
+ * so @a *different_p12 is set to 0.
+ */
+svn_error_t *
+svn_io_filesizes_three_different_p(svn_boolean_t *different_p12,
+ svn_boolean_t *different_p23,
+ svn_boolean_t *different_p13,
+ const char *file1,
+ const char *file2,
+ const char *file3,
+ apr_pool_t *scratch_pool);
/** Return in @a *checksum the checksum of type @a kind of @a file
* Use @a pool for temporary allocations and to allocate @a *checksum.
@@ -642,6 +661,20 @@ svn_io_files_contents_same_p(svn_boolean
const char *file2,
apr_pool_t *pool);
+/** Set @a *same12 to TRUE if @a file1 and @a file2 have the same
+ * contents, else set it to FALSE. Do the similar for @a *same23
+ * with @a file2 and @a file3, and @a *same13 for @a file1 and @a
+ * file3. Use @a pool for temporary allocations.
+ */
+svn_error_t *
+svn_io_files_contents_three_same_p(svn_boolean_t *same12,
+ svn_boolean_t *same23,
+ svn_boolean_t *same13,
+ const char *file1,
+ const char *file2,
+ const char *file3,
+ apr_pool_t *scratch_pool);
+
/** Create file at utf8-encoded @a file with contents @a contents.
* @a file must not already exist.
* Use @a pool for memory allocations.
Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1356261&r1=1356260&r2=1356261&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Mon Jul 2 14:34:11 2012
@@ -1311,6 +1311,43 @@ svn_io_filesizes_different_p(svn_boolean
svn_error_t *
+svn_io_filesizes_three_different_p(svn_boolean_t *different_p12,
+ svn_boolean_t *different_p23,
+ svn_boolean_t *different_p13,
+ const char *file1,
+ const char *file2,
+ const char *file3,
+ apr_pool_t *scratch_pool)
+{
+ apr_finfo_t finfo1, finfo2, finfo3;
+ apr_status_t status1, status2, status3;
+ const char *file1_apr, *file2_apr, *file3_apr;
+
+ /* Not using svn_io_stat() because don't want to generate
+ svn_error_t objects for non-error conditions. */
+
+ SVN_ERR(cstring_from_utf8(&file1_apr, file1, scratch_pool));
+ SVN_ERR(cstring_from_utf8(&file2_apr, file2, scratch_pool));
+ SVN_ERR(cstring_from_utf8(&file3_apr, file3, scratch_pool));
+
+ /* Stat all three files */
+ status1 = apr_stat(&finfo1, file1_apr, APR_FINFO_MIN, scratch_pool);
+ status2 = apr_stat(&finfo2, file2_apr, APR_FINFO_MIN, scratch_pool);
+ status3 = apr_stat(&finfo3, file3_apr, APR_FINFO_MIN, scratch_pool);
+
+ /* If we got an error stat'ing a file, it could be because the
+ file was removed... or who knows. Whatever the case, we
+ don't know if the filesizes are definitely different, so
+ assume that they're not. */
+ *different_p12 = !status1 && !status2 && finfo1.size != finfo2.size;
+ *different_p23 = !status2 && !status3 && finfo2.size != finfo3.size;
+ *different_p13 = !status1 && !status3 && finfo1.size != finfo3.size;
+
+ return SVN_NO_ERROR;
+}
+
+
+svn_error_t *
svn_io_file_checksum2(svn_checksum_t **checksum,
const char *file,
svn_checksum_kind_t kind,
@@ -4076,6 +4113,138 @@ contents_identical_p(svn_boolean_t *iden
+/* Do a byte-for-byte comparison of FILE1, FILE2 and FILE3. */
+static svn_error_t *
+contents_three_identical_p(svn_boolean_t *identical_p12,
+ svn_boolean_t *identical_p23,
+ svn_boolean_t *identical_p13,
+ const char *file1,
+ const char *file2,
+ const char *file3,
+ apr_pool_t *scratch_pool)
+{
+ svn_error_t *err;
+ apr_size_t bytes_read1, bytes_read2, bytes_read3;
+ char *buf1 = apr_palloc(scratch_pool, SVN__STREAM_CHUNK_SIZE);
+ char *buf2 = apr_palloc(scratch_pool, SVN__STREAM_CHUNK_SIZE);
+ char *buf3 = apr_palloc(scratch_pool, SVN__STREAM_CHUNK_SIZE);
+ apr_file_t *file1_h;
+ apr_file_t *file2_h;
+ apr_file_t *file3_h;
+ svn_boolean_t eof1 = FALSE;
+ svn_boolean_t eof2 = FALSE;
+ svn_boolean_t eof3 = FALSE;
+ svn_boolean_t read_1, read_2, read_3;
+
+ SVN_ERR(svn_io_file_open(&file1_h, file1, APR_READ, APR_OS_DEFAULT,
+ scratch_pool));
+
+ err = svn_io_file_open(&file2_h, file2, APR_READ, APR_OS_DEFAULT,
+ scratch_pool);
+
+ if (err)
+ return svn_error_trace(
+ svn_error_compose_create(err,
+ svn_io_file_close(file1_h, scratch_pool)));
+
+ err = svn_io_file_open(&file3_h, file3, APR_READ, APR_OS_DEFAULT,
+ scratch_pool);
+
+ if (err)
+ return svn_error_trace(
+ svn_error_compose_create(
+ err,
+ svn_error_compose_create(svn_io_file_close(file1_h,
+ scratch_pool),
+ svn_io_file_close(file2_h,
+ scratch_pool))));
+
+ /* assume TRUE, until disproved below */
+ *identical_p12 = *identical_p23 = *identical_p13 = TRUE;
+ /* We need to read as long as no error occurs, and as long as one of the
+ * flags could still change due to a read operation */
+ while (!err
+ && ((*identical_p12 && !eof1 && !eof2)
+ || (*identical_p23 && !eof2 && !eof3)
+ || (*identical_p13 && !eof1 && !eof3)))
+ {
+ read_1 = read_2 = read_3 = FALSE;
+
+ /* As long as a file is not at the end yet, and it is still
+ * potentially identical to another file, we read the next chunk.*/
+ if (!eof1 && (identical_p12 || identical_p13))
+ {
+ err = svn_io_file_read_full2(file1_h, buf1,
+ SVN__STREAM_CHUNK_SIZE, &bytes_read1,
+ &eof1, scratch_pool);
+ if (err)
+ break;
+ read_1 = TRUE;
+ }
+
+ if (!eof2 && (identical_p12 || identical_p23))
+ {
+ err = svn_io_file_read_full2(file2_h, buf2,
+ SVN__STREAM_CHUNK_SIZE, &bytes_read2,
+ &eof2, scratch_pool);
+ if (err)
+ break;
+ read_2 = TRUE;
+ }
+
+ if (!eof3 && (identical_p13 || identical_p23))
+ {
+ err = svn_io_file_read_full2(file3_h, buf3,
+ SVN__STREAM_CHUNK_SIZE, &bytes_read3,
+ &eof3, scratch_pool);
+ if (err)
+ break;
+ read_3 = TRUE;
+ }
+
+ /* If the files are still marked identical, and at least one of them
+ * is not at the end of file, we check whether they differ, and set
+ * their flag to false then. */
+ if (*identical_p12
+ && (read_1 || read_2)
+ && ((eof1 != eof2)
+ || (bytes_read1 != bytes_read2)
+ || memcmp(buf1, buf2, bytes_read1)))
+ {
+ *identical_p12 = FALSE;
+ }
+
+ if (*identical_p23
+ && (read_2 || read_3)
+ && ((eof2 != eof3)
+ || (bytes_read2 != bytes_read3)
+ || memcmp(buf2, buf3, bytes_read2)))
+ {
+ *identical_p23 = FALSE;
+ }
+
+ if (*identical_p13
+ && (read_1 || read_3)
+ && ((eof1 != eof3)
+ || (bytes_read1 != bytes_read3)
+ || memcmp(buf1, buf3, bytes_read3)))
+ {
+ *identical_p13 = FALSE;
+ }
+ }
+
+ return svn_error_trace(
+ svn_error_compose_create(
+ err,
+ svn_error_compose_create(
+ svn_io_file_close(file1_h, scratch_pool),
+ svn_error_compose_create(
+ svn_io_file_close(file2_h, scratch_pool),
+ svn_io_file_close(file3_h, scratch_pool)))));
+}
+
+
+
svn_error_t *
svn_io_files_contents_same_p(svn_boolean_t *same,
const char *file1,
@@ -4102,6 +4271,55 @@ svn_io_files_contents_same_p(svn_boolean
return SVN_NO_ERROR;
}
+svn_error_t *
+svn_io_files_contents_three_same_p(svn_boolean_t *same12,
+ svn_boolean_t *same23,
+ svn_boolean_t *same13,
+ const char *file1,
+ const char *file2,
+ const char *file3,
+ apr_pool_t *scratch_pool)
+{
+ svn_boolean_t diff_size12, diff_size23, diff_size13;
+
+ SVN_ERR(svn_io_filesizes_three_different_p(&diff_size12,
+ &diff_size23,
+ &diff_size13,
+ file1,
+ file2,
+ file3,
+ scratch_pool));
+
+ if (diff_size12 && diff_size23 && diff_size13)
+ {
+ *same12 = *same23 = *same13 = FALSE;
+ }
+ else if (diff_size12 && diff_size23)
+ {
+ *same12 = *same23 = FALSE;
+ SVN_ERR(contents_identical_p(same13, file1, file3, scratch_pool));
+ }
+ else if (diff_size23 && diff_size13)
+ {
+ *same23 = *same13 = FALSE;
+ SVN_ERR(contents_identical_p(same12, file1, file2, scratch_pool));
+ }
+ else if (diff_size12 && diff_size13)
+ {
+ *same12 = *same13 = FALSE;
+ SVN_ERR(contents_identical_p(same23, file2, file3, scratch_pool));
+ }
+ else
+ {
+ SVN_ERR_ASSERT(!diff_size12 && !diff_size23 && !diff_size13);
+ SVN_ERR(contents_three_identical_p(same12, same23, same13,
+ file1, file2, file3,
+ scratch_pool));
+ }
+
+ return SVN_NO_ERROR;
+}
+
#ifdef WIN32
/* Counter value of file_mktemp request (used in a threadsafe way), to make
sure that a single process normally never generates the same tempname
Modified: subversion/trunk/subversion/libsvn_wc/merge.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/merge.c?rev=1356261&r1=1356260&r2=1356261&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/merge.c (original)
+++ subversion/trunk/subversion/libsvn_wc/merge.c Mon Jul 2 14:34:11 2012
@@ -630,7 +630,9 @@ merge_file_trivial(svn_skel_t **work_ite
apr_pool_t *scratch_pool)
{
svn_skel_t *work_item;
- svn_boolean_t same_contents = FALSE;
+ svn_boolean_t same_left_right;
+ svn_boolean_t same_right_target;
+ svn_boolean_t same_left_target;
svn_node_kind_t kind;
svn_boolean_t is_special;
@@ -643,19 +645,23 @@ merge_file_trivial(svn_skel_t **work_ite
return SVN_NO_ERROR;
}
+ /* Check the files */
+ SVN_ERR(svn_io_files_contents_three_same_p(&same_left_right,
+ &same_right_target,
+ &same_left_target,
+ left_abspath,
+ right_abspath,
+ detranslated_target_abspath,
+ scratch_pool));
+
/* If the LEFT side of the merge is equal to WORKING, then we can
* copy RIGHT directly. */
- SVN_ERR(svn_io_files_contents_same_p(&same_contents, left_abspath,
- detranslated_target_abspath,
- scratch_pool));
- if (same_contents)
+ if (same_left_target)
{
/* Check whether the left side equals the right side.
* If it does, there is no change to merge so we leave the target
* unchanged. */
- SVN_ERR(svn_io_files_contents_same_p(&same_contents, left_abspath,
- right_abspath, scratch_pool));
- if (same_contents)
+ if (same_left_right)
{
*merge_outcome = svn_wc_merge_unchanged;
}
@@ -684,10 +690,7 @@ merge_file_trivial(svn_skel_t **work_ite
* conflicted them needlessly, while merge_text_file figured it out
* eventually and returned svn_wc_merge_unchanged for them, which
* is what we do here. */
- SVN_ERR(svn_io_files_contents_same_p(&same_contents,
- detranslated_target_abspath,
- right_abspath, scratch_pool));
- if (same_contents)
+ if (same_right_target)
{
*merge_outcome = svn_wc_merge_unchanged;
return SVN_NO_ERROR;
Propchange: subversion/trunk/subversion/tests/libsvn_subr/
------------------------------------------------------------------------------
--- svn:ignore (original)
+++ svn:ignore Mon Jul 2 14:34:11 2012
@@ -38,3 +38,5 @@ subst_translate-test
spillbuf-test
named_atomic-test
named_atomic-proc-test
+io-test
+io-test-temp
Added: subversion/trunk/subversion/tests/libsvn_subr/io-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/io-test.c?rev=1356261&view=auto
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_subr/io-test.c (added)
+++ subversion/trunk/subversion/tests/libsvn_subr/io-test.c Mon Jul 2 14:34:11 2012
@@ -0,0 +1,470 @@
+/* io-test.c --- tests for some i/o functions
+ *
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ */
+
+#include <stdlib.h>
+#include <stdarg.h>
+#include <string.h>
+#include <stdio.h>
+
+#include <apr.h>
+
+#include "svn_pools.h"
+#include "svn_string.h"
+#include "private/svn_skel.h"
+
+#include "../svn_test.h"
+#include "../svn_test_fs.h"
+
+
+/* Helpers to create the test data directory. */
+
+#define TEST_DIR "io-test-temp"
+
+/* The definition for the test data files. */
+struct test_file_definition_t
+ {
+ /* The name of the test data file. */
+ const char* const name;
+
+ /* The string needs to contain up to 5 bytes, they
+ * are interpreded as:
+ * - first byte
+ * - filler between first and medium byte
+ * - medium byte (the byte in the middle of the file)
+ * - filler between medium and last byte
+ * - last byte.
+ * If the string is shorter than the file length,
+ * the test will fail. */
+ const char* const data;
+
+ /* The size of the file actually to create. */
+ const apr_off_t size;
+
+ /* The created path of the file. Will be filled in
+ * by create_test_file() */
+ char* created_path;
+ };
+
+struct test_file_definition_t test_file_definitions[] =
+ {
+ {"empty", "", 0},
+ {"single_a", "a", 1},
+ {"single_b", "b", 1},
+ {"hundred_a", "aaaaa", 100},
+ {"hundred_b", "bbbbb", 100},
+ {"hundred_b1", "baaaa", 100},
+ {"hundred_b2", "abaaa", 100},
+ {"hundred_b3", "aabaa", 100},
+ {"hundred_b4", "aaaba", 100},
+ {"hundred_b5", "aaaab", 100},
+ {"chunk_minus_one_a", "aaaaa", SVN__STREAM_CHUNK_SIZE - 1},
+ {"chunk_minus_one_b1", "baaaa", SVN__STREAM_CHUNK_SIZE - 1},
+ {"chunk_minus_one_b2", "abaaa", SVN__STREAM_CHUNK_SIZE - 1},
+ {"chunk_minus_one_b3", "aabaa", SVN__STREAM_CHUNK_SIZE - 1},
+ {"chunk_minus_one_b4", "aaaba", SVN__STREAM_CHUNK_SIZE - 1},
+ {"chunk_minus_one_b5", "aaaab", SVN__STREAM_CHUNK_SIZE - 1},
+ {"chunk_a", "aaaaa", SVN__STREAM_CHUNK_SIZE},
+ {"chunk_b1", "baaaa", SVN__STREAM_CHUNK_SIZE},
+ {"chunk_b2", "abaaa", SVN__STREAM_CHUNK_SIZE},
+ {"chunk_b3", "aabaa", SVN__STREAM_CHUNK_SIZE},
+ {"chunk_b4", "aaaba", SVN__STREAM_CHUNK_SIZE},
+ {"chunk_b5", "aaaab", SVN__STREAM_CHUNK_SIZE},
+ {"chunk_plus_one_a", "aaaaa", SVN__STREAM_CHUNK_SIZE + 1},
+ {"chunk_plus_one_b1", "baaaa", SVN__STREAM_CHUNK_SIZE + 1},
+ {"chunk_plus_one_b2", "abaaa", SVN__STREAM_CHUNK_SIZE + 1},
+ {"chunk_plus_one_b3", "aabaa", SVN__STREAM_CHUNK_SIZE + 1},
+ {"chunk_plus_one_b4", "aaaba", SVN__STREAM_CHUNK_SIZE + 1},
+ {"chunk_plus_one_b5", "aaaab", SVN__STREAM_CHUNK_SIZE + 1},
+ {"twochunk_minus_one_a", "aaaaa", SVN__STREAM_CHUNK_SIZE*2 - 1},
+ {"twochunk_minus_one_b1", "baaaa", SVN__STREAM_CHUNK_SIZE*2 - 1},
+ {"twochunk_minus_one_b2", "abaaa", SVN__STREAM_CHUNK_SIZE*2 - 1},
+ {"twochunk_minus_one_b3", "aabaa", SVN__STREAM_CHUNK_SIZE*2 - 1},
+ {"twochunk_minus_one_b4", "aaaba", SVN__STREAM_CHUNK_SIZE*2 - 1},
+ {"twochunk_minus_one_b5", "aaaab", SVN__STREAM_CHUNK_SIZE*2 - 1},
+ {"twochunk_a", "aaaaa", SVN__STREAM_CHUNK_SIZE*2},
+ {"twochunk_b1", "baaaa", SVN__STREAM_CHUNK_SIZE*2},
+ {"twochunk_b2", "abaaa", SVN__STREAM_CHUNK_SIZE*2},
+ {"twochunk_b3", "aabaa", SVN__STREAM_CHUNK_SIZE*2},
+ {"twochunk_b4", "aaaba", SVN__STREAM_CHUNK_SIZE*2},
+ {"twochunk_b5", "aaaab", SVN__STREAM_CHUNK_SIZE*2},
+ {"twochunk_plus_one_a", "aaaaa", SVN__STREAM_CHUNK_SIZE*2 + 1},
+ {"twochunk_plus_one_b1", "baaaa", SVN__STREAM_CHUNK_SIZE*2 + 1},
+ {"twochunk_plus_one_b2", "abaaa", SVN__STREAM_CHUNK_SIZE*2 + 1},
+ {"twochunk_plus_one_b3", "aabaa", SVN__STREAM_CHUNK_SIZE*2 + 1},
+ {"twochunk_plus_one_b4", "aaaba", SVN__STREAM_CHUNK_SIZE*2 + 1},
+ {"twochunk_plus_one_b5", "aaaab", SVN__STREAM_CHUNK_SIZE*2 + 1},
+ {0},
+ };
+
+/* Function to prepare a single test file */
+
+static svn_error_t *
+create_test_file(struct test_file_definition_t* definition,
+ apr_pool_t *pool,
+ apr_pool_t *scratch_pool)
+{
+ apr_status_t status = 0;
+ apr_file_t *file_h;
+ apr_off_t midpos = definition->size / 2;
+ svn_error_t *err = NULL;
+ int i;
+
+ if (definition->size < 5)
+ SVN_ERR_ASSERT(strlen(definition->data) >= definition->size);
+ else
+ SVN_ERR_ASSERT(strlen(definition->data) >= 5);
+
+
+ definition->created_path = svn_dirent_join(TEST_DIR,
+ definition->name,
+ pool);
+
+ SVN_ERR(svn_io_file_open(&file_h,
+ definition->created_path,
+ APR_FOPEN_WRITE | APR_FOPEN_CREATE | APR_FOPEN_EXCL,
+ APR_OS_DEFAULT,
+ scratch_pool));
+
+ for (i=1; i <= definition->size; i += 1)
+ {
+ char c;
+ if (i == 1)
+ c = definition->data[0];
+ else if (i < midpos)
+ c = definition->data[1];
+ else if (i == midpos)
+ c = definition->data[2];
+ else if (i < definition->size)
+ c = definition->data[3];
+ else
+ c = definition->data[4];
+
+ status = apr_file_putc(c, file_h);
+
+ if (status)
+ break;
+ }
+
+ if (status)
+ err = svn_error_wrap_apr(status, "Can't write to file '%s'",
+ definition->name);
+
+ return svn_error_compose_create(err,
+ svn_io_file_close(file_h, scratch_pool));
+}
+
+/* Function to prepare the whole set of on-disk files to be compared. */
+static svn_error_t *
+create_comparison_candidates(apr_pool_t *scratch_pool)
+{
+ svn_node_kind_t kind;
+ apr_pool_t *iterpool = svn_pool_create(scratch_pool);
+ struct test_file_definition_t *candidate;
+ svn_error_t *err = SVN_NO_ERROR;
+
+ /* If there's already a directory named io-test-temp, delete it.
+ Doing things this way means that repositories stick around after
+ a failure for postmortem analysis, but also that tests can be
+ re-run without cleaning out the repositories created by prior
+ runs. */
+ SVN_ERR(svn_io_check_path(TEST_DIR, &kind, scratch_pool));
+
+ if (kind == svn_node_dir)
+ SVN_ERR(svn_io_remove_dir2(TEST_DIR, TRUE, NULL, NULL, scratch_pool));
+ else if (kind != svn_node_none)
+ return svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
+ "There is already a file named '%s'",
+ TEST_DIR);
+
+ SVN_ERR(svn_io_dir_make(TEST_DIR, APR_OS_DEFAULT, scratch_pool));
+
+ svn_test_add_dir_cleanup(TEST_DIR);
+
+ for (candidate = test_file_definitions;
+ candidate->name != NULL;
+ candidate += 1)
+ {
+ svn_pool_clear(iterpool);
+ err = create_test_file(candidate, scratch_pool, iterpool);
+ if (err)
+ break;
+ }
+
+ svn_pool_destroy(iterpool);
+
+ return err;
+}
+
+
+/* Functions to check the 2-way and 3-way file comparison functions. */
+
+/* Test 2-way file size checking */
+static svn_error_t *
+test_two_file_size_comparison(apr_pool_t *scratch_pool)
+{
+ struct test_file_definition_t *inner, *outer;
+ svn_boolean_t actual;
+ svn_boolean_t expected;
+ svn_error_t *err = SVN_NO_ERROR;
+ svn_error_t *cmp_err;
+ apr_pool_t *iterpool = svn_pool_create(scratch_pool);
+
+ SVN_ERR(create_comparison_candidates(scratch_pool));
+
+ for (outer = test_file_definitions; outer->name != NULL; outer += 1)
+ {
+ for (inner = test_file_definitions; inner->name != NULL; inner += 1)
+ {
+ svn_pool_clear(iterpool);
+
+ expected = inner->size != outer->size;
+
+ cmp_err = svn_io_filesizes_different_p(&actual,
+ inner->created_path,
+ outer->created_path,
+ iterpool);
+
+ if (cmp_err)
+ {
+ err = svn_error_compose_create(err, cmp_err);
+ }
+ else if (expected != actual)
+ {
+ err = svn_error_compose_create(err,
+ svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
+ "size comparison problem: '%s' and '%s'",
+ inner->created_path,
+ outer->created_path));
+ }
+ }
+ }
+
+ svn_pool_destroy(iterpool);
+ return err;
+}
+
+
+/* Test 2-way file content checking */
+static svn_error_t *
+test_two_file_content_comparison(apr_pool_t *scratch_pool)
+{
+ struct test_file_definition_t *inner, *outer;
+ svn_boolean_t actual;
+ svn_boolean_t expected;
+ svn_error_t *err = SVN_NO_ERROR;
+ svn_error_t *cmp_err;
+ apr_pool_t *iterpool = svn_pool_create(scratch_pool);
+
+ SVN_ERR(create_comparison_candidates(scratch_pool));
+
+ for (outer = test_file_definitions; outer->name != NULL; outer += 1)
+ {
+ for (inner = test_file_definitions; inner->name != NULL; inner += 1)
+ {
+ svn_pool_clear(iterpool);
+
+ expected = inner->size == outer->size
+ && strcmp(inner->data, outer->data) == 0;
+
+ cmp_err = svn_io_files_contents_same_p(&actual,
+ inner->created_path,
+ outer->created_path,
+ iterpool);
+
+ if (cmp_err)
+ {
+ err = svn_error_compose_create(err, cmp_err);
+ }
+ else
+ {
+ if (expected != actual)
+ err = svn_error_compose_create(err,
+ svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
+ "content comparison problem: '%s' and '%s'",
+ inner->created_path,
+ outer->created_path));
+ }
+ }
+ }
+
+ svn_pool_destroy(iterpool);
+ return err;
+}
+
+
+/* Test 3-way file size checking */
+static svn_error_t *
+test_three_file_size_comparison(apr_pool_t *scratch_pool)
+{
+ struct test_file_definition_t *inner, *middle, *outer;
+ svn_boolean_t actual12, actual23, actual13;
+ svn_boolean_t expected12, expected23, expected13;
+ svn_error_t *err = SVN_NO_ERROR;
+ svn_error_t *cmp_err;
+ apr_pool_t *iterpool = svn_pool_create(scratch_pool);
+
+ SVN_ERR(create_comparison_candidates(scratch_pool));
+
+ for (outer = test_file_definitions; outer->name != NULL; outer += 1)
+ {
+ for (middle = test_file_definitions; middle->name != NULL; middle += 1)
+ {
+ for (inner = test_file_definitions; inner->name != NULL; inner += 1)
+ {
+ svn_pool_clear(iterpool);
+
+ expected12 = inner->size != middle->size;
+ expected23 = middle->size != outer->size;
+ expected13 = inner->size != outer->size;
+
+ cmp_err = svn_io_filesizes_three_different_p(&actual12,
+ &actual23,
+ &actual13,
+ inner->created_path,
+ middle->created_path,
+ outer->created_path,
+ iterpool);
+
+ if (cmp_err)
+ {
+ err = svn_error_compose_create(err, cmp_err);
+ }
+ else
+ {
+ if (expected12 != actual12)
+ err = svn_error_compose_create(err,
+ svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
+ "size comparison problem: '%s' and '%s'",
+ inner->created_path,
+ middle->created_path));
+
+ if (expected23 != actual23)
+ err = svn_error_compose_create(err,
+ svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
+ "size comparison problem: '%s' and '%s'",
+ middle->created_path,
+ outer->created_path));
+
+ if (expected13 != actual13)
+ err = svn_error_compose_create(err,
+ svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
+ "size comparison problem: '%s' and '%s'",
+ inner->created_path,
+ outer->created_path));
+ }
+ }
+ }
+ }
+
+ svn_pool_destroy(iterpool);
+
+ return err;
+}
+
+
+/* Test 3-way file content checking */
+static svn_error_t *
+test_three_file_content_comparison(apr_pool_t *scratch_pool)
+{
+ struct test_file_definition_t *inner, *middle, *outer;
+ svn_boolean_t actual12, actual23, actual13;
+ svn_boolean_t expected12, expected23, expected13;
+ svn_error_t *err = SVN_NO_ERROR;
+ svn_error_t *cmp_err;
+ apr_pool_t *iterpool = svn_pool_create(scratch_pool);
+
+ SVN_ERR(create_comparison_candidates(scratch_pool));
+
+ for (outer = test_file_definitions; outer->name != NULL; outer += 1)
+ {
+ for (middle = test_file_definitions; middle->name != NULL; middle += 1)
+ {
+ for (inner = test_file_definitions; inner->name != NULL; inner += 1)
+ {
+ svn_pool_clear(iterpool);
+
+ expected12 = outer->size == middle->size
+ && strcmp(outer->data, middle->data) == 0;
+ expected23 = middle->size == inner->size
+ && strcmp(middle->data, inner->data) == 0;
+ expected13 = outer->size == inner->size
+ && strcmp(outer->data, inner->data) == 0;
+
+ cmp_err = svn_io_files_contents_three_same_p(&actual12,
+ &actual23,
+ &actual13,
+ outer->created_path,
+ middle->created_path,
+ inner->created_path,
+ iterpool);
+
+ if (cmp_err)
+ {
+ err = svn_error_compose_create(err, cmp_err);
+ }
+ else
+ {
+ if (expected12 != actual12)
+ err = svn_error_compose_create(err,
+ svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
+ "size comparison problem: '%s' and '%s'",
+ inner->created_path,
+ middle->created_path));
+
+ if (expected23 != actual23)
+ err = svn_error_compose_create(err,
+ svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
+ "size comparison problem: '%s' and '%s'",
+ middle->created_path,
+ outer->created_path));
+
+ if (expected13 != actual13)
+ err = svn_error_compose_create(err,
+ svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
+ "size comparison problem: '%s' and '%s'",
+ inner->created_path,
+ outer->created_path));
+ }
+ }
+ }
+ }
+
+ return err;
+}
+
+
+
+/* The test table. */
+
+struct svn_test_descriptor_t test_funcs[] =
+ {
+ SVN_TEST_NULL,
+ SVN_TEST_PASS2(test_two_file_size_comparison,
+ "two file size comparison"),
+ SVN_TEST_PASS2(test_two_file_content_comparison,
+ "two file content comparison"),
+ SVN_TEST_PASS2(test_three_file_size_comparison,
+ "three file size comparison"),
+ SVN_TEST_PASS2(test_three_file_content_comparison,
+ "three file content comparison"),
+ SVN_TEST_NULL
+ };
Propchange: subversion/trunk/subversion/tests/libsvn_subr/io-test.c
------------------------------------------------------------------------------
svn:eol-style = native
RE: svn commit: r1356261 - in /subversion/trunk: build.conf subversion/include/svn_io.h subversion/libsvn_subr/io.c subversion/libsvn_wc/merge.c subversion/tests/libsvn_subr/ subversion/tests/libsvn_subr/io-test.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: philip@apache.org [mailto:philip@apache.org]
> Sent: maandag 2 juli 2012 16:34
> To: commits@subversion.apache.org
> Subject: svn commit: r1356261 - in /subversion/trunk: build.conf
> subversion/include/svn_io.h subversion/libsvn_subr/io.c
> subversion/libsvn_wc/merge.c subversion/tests/libsvn_subr/
> subversion/tests/libsvn_subr/io-test.c
>
> Author: philip
> Date: Mon Jul 2 14:34:11 2012
> New Revision: 1356261
>
> URL: http://svn.apache.org/viewvc?rev=1356261&view=rev
> Log:
> Optimize merge_file_trivial() by avoiding to read the files twice by
> using a new comparison function which compares 3 files at once. Also
> add C tests for the new and existing file comparison functions in
> libsvn_subr/io.c
>
> * 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.
>
> * build.conf
> (io-test): Add the new io-test.c file to the build configuration.
>
> * subversion/tests/libsvn_subr:
> Add the executable and temporary test file folder to svn:ignore.
>
> * subversion/tests/libsvn_subr/io-test.c
> (create_test_file): Helper function to create a test data file.
> (create_comparison_candidates): Helper function to create the full
> set of test data files.
> (test_two_file_size_comparison): Test function for
> svn_io_filesizes_different_p.
> (test_two_file_content_comparison): Test function for
> svn_io_files_contents_same_p.
> (test_three_file_size_comparison): Test function for
> test_three_file_size_comparison.
> (test_three_file_content_comparison): Test function for
> svn_io_files_contents_three_same_p.
Each of these functions performs 46*46*46 = +- 97000 file comparisons.
So that adds up to a total of rougly 300000 comparisons.
This takes +- 6% of the total testsuite time on my ramdrive setup (22 seconds). Probably much larger on most other systems.
We should really simplify this function as it is very unlikely that it really catches that much errors on every testrun.
It is a nice test for incidental testing, but shouldn't be part of the standard testrun in its current form.
Bert