On Sat, Feb 7, 2009 at 8:23 AM, Bert Huijben <rhuijben_at_sharpsvn.net> wrote:
> Author: rhuijben
> Date: Sat Feb 7 05:23:44 2009
> New Revision: 35733
>
> Log:
> * subversion/bindings/javahl/native/JNIUtil.cpp
> (JNIUtil::preprocessPath): Remove a gigantic Windows only performance
> penalty by using the always correct svn_path_internal_style() instead
> of the hidden breaking apr_filepath_merge with APR_FILEPATH_TRUENAME
> that only did validation on Windows.
>
> Reported on users@ as: Linux is 10 times faster than Windows because Eclipse
> is slow on Windows.
>
> Modified:
> trunk/subversion/bindings/javahl/native/JNIUtil.cpp
>
> Modified: trunk/subversion/bindings/javahl/native/JNIUtil.cpp
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/bindings/javahl/native/JNIUtil.cpp?pathrev=35733&r1=35732&r2=35733
> ==============================================================================
> --- trunk/subversion/bindings/javahl/native/JNIUtil.cpp Sat Feb 7 05:04:28 2009 (r35732)
> +++ trunk/subversion/bindings/javahl/native/JNIUtil.cpp Sat Feb 7 05:23:44 2009 (r35733)
> @@ -786,38 +786,31 @@ svn_error_t *JNIUtil::preprocessPath(con
> return svn_error_createf(SVN_ERR_BAD_URL, NULL,
> _("URL '%s' contains a '..' element"),
> path);
> -
> - /* strip any trailing '/' */
> - path = svn_path_canonicalize(path, pool);
> }
> else /* not a url, so treat as a path */
> {
> - const char *apr_target;
> - char *truenamed_target; /* APR-encoded */
> - apr_status_t apr_err;
> + /* Normalize path to subversion internal style */
>
> - /* canonicalize case, and change all separators to '/'. */
> - SVN_ERR(svn_path_cstring_from_utf8(&apr_target, path, pool));
> - apr_err = apr_filepath_merge(&truenamed_target, "", apr_target,
> - APR_FILEPATH_TRUENAME, pool);
> + /* ### In Subversion < 1.6 this method on Windows actually tried
> + to lookup the path on disk to fix possible invalid casings in
> + the passed path. (An extremely expensive operation; especially
> + on network drives).
>
> - if (!apr_err)
> - /* We have a canonicalized APR-encoded target now. */
> - apr_target = truenamed_target;
> - else if (APR_STATUS_IS_ENOENT(apr_err))
> - /* It's okay for the file to not exist, that just means we
> - have to accept the case given to the client. We'll use
> - the original APR-encoded target. */
> - ;
> - else
> - return svn_error_createf(apr_err, NULL,
> - _("Error resolving case of '%s'"),
> - svn_path_local_style(path, pool));
> + This 'feature'is now removed as it penalizes every correct
> + path passed, and also breaks behavior of e.g.
> + 'svn status .' returns '!' file, because there is only a "File"
> + on disk.
> + But when you then call 'svn status file', you get '? File'.
>
> - /* convert back to UTF-8. */
> - SVN_ERR(svn_path_cstring_to_utf8(&path, apr_target, pool));
> - path = svn_path_canonicalize(path, pool);
> + As JavaHL is designed to be platform independent I assume users
> + don't want this broken behavior on non round-trippable paths, nor
> + the performance penalty.
> + */
>
> + path = svn_path_internal_style(path, pool);
Hi Bert,
A minor nit: svn_path_internal_style() already calls svn_path_canonicalize()...
> }
> + /* strip any trailing '/' */
> + path = svn_path_canonicalize(path, pool);
...so this call to svn_path_canonicalize() can be removed and the call
in the 'if (svn_path_is_url(path))' block you removed put back.
Paul
> +
> return NULL;
> }
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=1119032
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1187449
Received on 2009-02-18 19:26:39 CET