This is a patch for discussion which I submitted to issue 3719
(“Extremely slow checkout on Windows”). I shan't repeat everything that's
there; essentially it fixes *really* slow checkouts to NTFS of directories
with large numbers of files with properties.
I originally modified it slightly to address my second comment on that
issue, namely that just using rand() will leave it with slightly less
functionality than the previous 99999 loop.
However, that was relying upon my Linux man page telling me RAND_MAX was
32767; it's actually 2147483647. I think it's still smaller on Windows,
though, so it would suffer that limitation. Should we include a small,
custom int-max rand() just for this?
Otherwise it might be a #if on RAND_MAX.
A supplementary question is: should I worry about where rand() is
included from? It “just worked” on my Linux build, but I've not tried for
Windows, etc.
Apologies if I haven't formatted all this correctly.
[[[
Fix issue #3719: prevent logarithmic slow down when checking out large
directories onto Windows NTFS
* subversion/libsvn_subr/io.c
svn_io_open_unique_file3: don't call svn_io_open_uniquely_named(), but
instead use a copy of that routine that uses rand() to get the next
potential free number instead if simply incrementing, minimising clashes on
repeated calls.
]]]
--
[neil@fnx ~]# rm -f .signature
[neil@fnx ~]# ls -l .signature
ls: .signature: No such file or directory
[neil@fnx ~]# exit
Index: subversion/libsvn_subr/io.c
===================================================================
--- subversion/libsvn_subr/io.c (revision 1066367)
+++ subversion/libsvn_subr/io.c (working copy)
@@ -466,11 +466,140 @@
apr_pool_t *result_pool,
apr_pool_t *scratch_pool)
{
- return svn_io_open_uniquely_named(file, temp_path,
- dirpath, "tempfile", ".tmp",
- delete_when, result_pool, scratch_pool);
+ const char *path;
+ unsigned int i;
+ struct temp_file_cleanup_s *baton = NULL;
+ char *filename = "tempfile";
+ char *suffix = ".tmp";
+
+ SVN_ERR_ASSERT(file || temp_path);
+
+ if (dirpath == NULL)
+ SVN_ERR(svn_io_temp_dir(&dirpath, scratch_pool));
+ if (filename == NULL)
+ filename = "tempfile";
+ if (suffix == NULL)
+ suffix = ".tmp";
+
+ path = svn_path_join(dirpath, filename, scratch_pool);
+
+ if (delete_when == svn_io_file_del_on_pool_cleanup)
+ {
+ baton = apr_palloc(result_pool, sizeof(*baton));
+
+ baton->pool = result_pool;
+ baton->name = NULL;
+
+ /* Because cleanups are run LIFO, we need to make sure to register
+ our cleanup before the apr_file_close cleanup:
+
+ On Windows, you can't remove an open file.
+ */
+ apr_pool_cleanup_register(result_pool, baton,
+ temp_file_plain_cleanup_handler,
+ temp_file_child_cleanup_handler);
}
+ for (i = 1; i <= RAND_MAX; i++)
+ {
+ const char *unique_name;
+ const char *unique_name_apr;
+ apr_file_t *try_file;
+ apr_status_t apr_err;
+ apr_int32_t flag = (APR_READ | APR_WRITE | APR_CREATE | APR_EXCL
+ | APR_BUFFERED | APR_BINARY);
+
+ if (delete_when == svn_io_file_del_on_close)
+ flag |= APR_DELONCLOSE;
+
+ /* Special case the first attempt -- if we can avoid having a
+ generated numeric portion at all, that's best. So first we
+ try with just the suffix; then future tries add a number
+ before the suffix. (A do-while loop could avoid the repeated
+ conditional, but it's not worth the clarity loss.) */
+ if (i == 1)
+ unique_name = apr_psprintf(scratch_pool, "%s%s", path, suffix);
+ else
+ {
+ /* Use rand instead of counting up from 1 to prevent logarithmic
+ * slowdown with large directory checkouts when continually looking
+ * for new temp files for the files' properties. */
+ unsigned int ir = rand();
+ unique_name = apr_psprintf(scratch_pool, "%s.%u%s", path, ir, suffix);
+ }
+
+ /* Hmmm. Ideally, we would append to a native-encoding buf
+ before starting iteration, then convert back to UTF-8 for
+ return. But I suppose that would make the appending code
+ sensitive to i18n in a way it shouldn't be... Oh well. */
+ SVN_ERR(cstring_from_utf8(&unique_name_apr, unique_name, scratch_pool));
+
+ apr_err = file_open(&try_file, unique_name_apr, flag,
+ APR_OS_DEFAULT, FALSE, result_pool);
+
+ if (APR_STATUS_IS_EEXIST(apr_err))
+ continue;
+ else if (apr_err)
+ {
+ /* On Win32, CreateFile fails with an "Access Denied" error
+ code, rather than "File Already Exists", if the colliding
+ name belongs to a directory. */
+ if (APR_STATUS_IS_EACCES(apr_err))
+ {
+ apr_finfo_t finfo;
+ apr_status_t apr_err_2 = apr_stat(&finfo, unique_name_apr,
+ APR_FINFO_TYPE, scratch_pool);
+
+ if (!apr_err_2 && finfo.filetype == APR_DIR)
+ continue;
+
+#if WIN32
+ apr_err_2 = APR_TO_OS_ERROR(apr_err);
+
+ if (apr_err_2 == ERROR_ACCESS_DENIED ||
+ apr_err_2 == ERROR_SHARING_VIOLATION)
+ {
+ /* The file is in use by another process or is hidden;
+ create a new name, but don't do this 99999 times in
+ case the folder is not writable */
+ i += 797;
+ continue;
+ }
+#endif
+
+ /* Else fall through and return the original error. */
+ }
+
+ if (file) *file = NULL;
+ if (temp_path) *temp_path = NULL;
+ return svn_error_wrap_apr(apr_err, _("Can't open '%s'"),
+ svn_path_local_style(unique_name,
+ scratch_pool));
+ }
+ else
+ {
+ if (delete_when == svn_io_file_del_on_pool_cleanup)
+ baton->name = apr_pstrdup(result_pool, unique_name_apr);
+
+ if (file)
+ *file = try_file;
+ else
+ apr_file_close(try_file);
+ if (temp_path)
+ *temp_path = apr_pstrdup(result_pool, unique_name);
+
+ return SVN_NO_ERROR;
+ }
+ }
+
+ if (file) *file = NULL;
+ if (temp_path) *temp_path = NULL;
+ return svn_error_createf(SVN_ERR_IO_UNIQUE_NAMES_EXHAUSTED,
+ NULL,
+ _("Unable to make name for '%s'"),
+ svn_path_local_style(path, scratch_pool));
+}
+
svn_error_t *
svn_io_create_unique_link(const char **unique_name_p,
const char *path,