You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2013/10/20 23:35:23 UTC

svn commit: r1533994 - in /subversion/trunk/subversion/libsvn_subr: cmdline.c win32_xlate.c win32_xlate.h

Author: rhuijben
Date: Sun Oct 20 21:35:23 2013
New Revision: 1533994

URL: http://svn.apache.org/r1533994
Log:
Add a Windows specific svn_cmdline_puts() shortcut for the direct console
output case to avoid the insanely expensive double character conversion
in the Visual C++ CRT for this specific, but for Subversion very common case.

Before:                              VM/Network WC  Host
$ svn status -v trunk              = 7.5s           5.8s
$ svn status -v trunk > file       = 0.7s           0.1s

After:
$ svn status -v trunk              = 1.3s           0.4s
$ svn status -v trunk > file       = 0.7s           0.1s

(Measured on Windows 7, 8 and 8.1 with several CRT versions >= 2008)

When locales are explicitly enabled (such as in Subversion) the Visual
C++ CRT applies a double character conversion to make the console output
identical to what DOS and Win9X would do, while the console natively supports
displaying unicode. Instead of using the standard conversion route, this
shortcut converts our utf-8 to utf-16 and sends that to the attached console.

This patch avoids the extreme performance penalty and at the same time
enables displaying all unicode characters that the console support even
when they can't be expressed in the ansi character set.

Note that you will not see any change in the test suite, as the test suite
redirects all output. It will make subversion easier to profile though,
as before this patch many performance scenarios were console io bound on
Windows.

* subversion/libsvn_subr/cmdline.c
  (includes): Add conio.h on Windows. Add win32_xlate.
  (svn_cmdline_init): Detect if stdout and/or stderr are connected to the
    console.
  (svn_cmdline_fputs): Shortcut stdout/stderr to the console when possible.

* subversion/libsvn_subr/win32_xlate.c
  (includes): Add svn_private_config.h.
  (svn_subr__win32_utf8_to_utf16): New function.

* subversion/libsvn_subr/win32_xlate.h
  (svn_subr__win32_utf8_to_utf16): New function.

Modified:
    subversion/trunk/subversion/libsvn_subr/cmdline.c
    subversion/trunk/subversion/libsvn_subr/win32_xlate.c
    subversion/trunk/subversion/libsvn_subr/win32_xlate.h

Modified: subversion/trunk/subversion/libsvn_subr/cmdline.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cmdline.c?rev=1533994&r1=1533993&r2=1533994&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/cmdline.c (original)
+++ subversion/trunk/subversion/libsvn_subr/cmdline.c Sun Oct 20 21:35:23 2013
@@ -34,6 +34,7 @@
 #else
 #include <crtdbg.h>
 #include <io.h>
+#include <conio.h>
 #endif
 
 #include <apr.h>                /* for STDIN_FILENO */
@@ -66,6 +67,7 @@
 #include "private/svn_string_private.h"
 
 #include "win32_crashrpt.h"
+#include "win32_xlate.h"
 
 #if defined(WIN32) && defined(_MSC_VER) && (_MSC_VER < 1400)
 /* Before Visual Studio 2005, the C runtime didn't handle encodings for the
@@ -77,6 +79,14 @@ static const char *input_encoding = NULL
 
 /* The stdout encoding. If null, it's the same as the native encoding. */
 static const char *output_encoding = NULL;
+#elif defined(WIN32) && defined(_MSC_VER)
+/* For now limit this code to Visual C++, as the result is highly dependent
+   on the CRT implementation */
+#define USE_WIN32_CONSOLE_SHORTCUT
+
+/* When TRUE, stdout/stderr is directly connected to a console */
+static svn_boolean_t shortcut_stdout_to_console = FALSE;
+static svn_boolean_t shortcut_stderr_to_console = FALSE;
 #endif
 
 
@@ -254,6 +264,31 @@ svn_cmdline_init(const char *progname, F
       return EXIT_FAILURE;
     }
 
+#ifdef USE_WIN32_CONSOLE_SHORTCUT
+  if (_isatty(STDOUT_FILENO))
+    {
+      DWORD ignored;
+      HANDLE stdout_handle = GetStdHandle(STD_OUTPUT_HANDLE);
+
+       /* stdout is a char device handle, but is it the console? */
+       if (GetConsoleMode(stdout_handle, &ignored))
+        shortcut_stdout_to_console = TRUE;
+
+       /* Don't close stdout_handle */
+    }
+  if (_isatty(STDERR_FILENO))
+    {
+      DWORD ignored;
+      HANDLE stderr_handle = GetStdHandle(STD_ERROR_HANDLE);
+
+       /* stderr is a char device handle, but is it the console? */
+      if (GetConsoleMode(stderr_handle, &ignored))
+          shortcut_stderr_to_console = TRUE;
+
+      /* Don't close stderr_handle */
+    }
+#endif
+
   return EXIT_SUCCESS;
 }
 
@@ -347,6 +382,45 @@ svn_cmdline_fputs(const char *string, FI
   svn_error_t *err;
   const char *out;
 
+#ifdef USE_WIN32_CONSOLE_SHORTCUT
+  /* For legacy reasons the Visual C++ runtime converts output to the console
+     from the native 'ansi' encoding, to unicode, then back to 'ansi' and then
+     onwards to the console which is implemented as unicode.
+
+     For operations like 'svn status -v' this may cause about 70% of the total
+     processing time, with absolutely no gain.
+
+     For this specific scenario this shortcut exists. It has the nice side
+     effect of allowing full unicode output to the console.
+
+     Note that this shortcut is not used when the output is redirected, as in
+     that case the data is put on the pipe/file after the first conversion to
+     ansi. In this case the most expensive conversion is already avoided.
+   */
+  if ((stream == stdout && shortcut_stdout_to_console)
+      || (stream == stderr && shortcut_stderr_to_console))
+    {
+      WCHAR *result;
+
+      if (string[0] == '\0')
+        return SVN_NO_ERROR;
+
+      SVN_ERR(svn_cmdline_fflush(stream)); /* Flush existing output */
+
+      SVN_ERR(svn_subr__win32_utf8_to_utf16(&result, string, pool));
+
+      if (_cputws(result))
+        {
+          if (apr_get_os_error())
+          {
+            return svn_error_wrap_apr(apr_get_os_error(), _("Write error"));
+          }
+        }
+
+      return SVN_NO_ERROR;
+    }
+#endif
+
   err = svn_cmdline_cstring_from_utf8(&out, string, pool);
 
   if (err)

Modified: subversion/trunk/subversion/libsvn_subr/win32_xlate.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/win32_xlate.c?rev=1533994&r1=1533993&r2=1533994&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/win32_xlate.c (original)
+++ subversion/trunk/subversion/libsvn_subr/win32_xlate.c Sun Oct 20 21:35:23 2013
@@ -50,6 +50,8 @@ typedef int win32_xlate__dummy;
 
 #include "win32_xlate.h"
 
+#include "svn_private_config.h"
+
 static svn_atomic_t com_initialized = 0;
 
 /* Initializes COM and keeps COM available until process exit.
@@ -235,4 +237,34 @@ svn_subr__win32_xlate_to_stringbuf(win32
   return APR_SUCCESS;
 }
 
+svn_error_t *
+svn_subr__win32_utf8_to_utf16(const WCHAR **result,
+                              const char *src,
+                              apr_pool_t *result_pool)
+{
+  WCHAR * wide_str;
+  int retval, wide_count;
+
+  retval = MultiByteToWideChar(CP_UTF8, 0, src, -1, NULL, 0);
+
+  if (retval == 0)
+    return svn_error_wrap_apr(apr_get_os_error(),
+                              _("Conversion to UTF-16 failed"));
+
+  wide_count = retval + 1;
+  wide_str = apr_palloc(result_pool, wide_count * sizeof(WCHAR));
+  wide_str[wide_count] = 0;
+
+  retval = MultiByteToWideChar(CP_UTF8, 0, src, -1, wide_str, wide_count);
+
+  if (retval == 0)
+    return svn_error_wrap_apr(apr_get_os_error(),
+                              _("Conversion to UTF-16 failed"));
+
+  *result = wide_str;
+
+  return SVN_NO_ERROR;
+}
+
+
 #endif /* WIN32 */

Modified: subversion/trunk/subversion/libsvn_subr/win32_xlate.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/win32_xlate.h?rev=1533994&r1=1533993&r2=1533994&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/win32_xlate.h (original)
+++ subversion/trunk/subversion/libsvn_subr/win32_xlate.h Sun Oct 20 21:35:23 2013
@@ -47,6 +47,12 @@ apr_status_t svn_subr__win32_xlate_to_st
                                                 svn_stringbuf_t **dest,
                                                 apr_pool_t *pool);
 
+/* On Windows: Convert the utf-8 string SRC to utf-16/ucs2. */
+svn_error_t *
+svn_subr__win32_utf8_to_utf16(const WCHAR **result,
+                              const char *src,
+                              apr_pool_t *result_pool);
+
 #endif /* WIN32 */
 
 #endif /* SVN_LIBSVN_SUBR_WIN32_XLATE_H */



Re: svn commit: r1533994 - in /subversion/trunk/subversion/libsvn_subr: cmdline.c win32_xlate.c win32_xlate.h

Posted by Branko Čibej <br...@wandisco.com>.
On 21.10.2013 07:56, Branko Čibej wrote:
> On 21.10.2013 07:48, Branko Čibej wrote:
>> On 20.10.2013 23:35, rhuijben@apache.org wrote:
>>> * subversion/libsvn_subr/cmdline.c
>>>   (includes): Add conio.h on Windows. Add win32_xlate.
>>>   (svn_cmdline_init): Detect if stdout and/or stderr are connected to the
>>>     console.
>>>   (svn_cmdline_fputs): Shortcut stdout/stderr to the console when possible.
>>>
>>> * subversion/libsvn_subr/win32_xlate.c
>>>   (includes): Add svn_private_config.h.
>>>   (svn_subr__win32_utf8_to_utf16): New function.
>> Good call!
>>
>> Can you now please go through all the places (two or three, I think)
>> where we're using the internal APR function that does the same thing,
>> and use this one instead; then we can finally make the Windows build
>> independent of APR sources.
> $ grep -r arch/win32 subversion
> subversion/bindings/javahl/native/JNIUtil.cpp:#include <arch/win32/apr_arch_utf8.h>
> subversion/libsvn_subr/io.c:#include <arch/win32/apr_arch_file_io.h>
>
>
> Both places use apr_conv_ucs2_to_utf8 and the inverse function, but io.c
> also uses some internal APR logic to tell the difference between WInNT
> and Win95. I'd vote for simply ripping any pending support for the
> ancient non-Unicode Windows OS so that we can get rid of that, too.

I've done the bit in JNIUtil.cpp, apparently successfully.

-- Brane


-- 
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com

Re: svn commit: r1533994 - in /subversion/trunk/subversion/libsvn_subr: cmdline.c win32_xlate.c win32_xlate.h

Posted by Branko Čibej <br...@wandisco.com>.
On 21.10.2013 07:48, Branko Čibej wrote:
> On 20.10.2013 23:35, rhuijben@apache.org wrote:
>> * subversion/libsvn_subr/cmdline.c
>>   (includes): Add conio.h on Windows. Add win32_xlate.
>>   (svn_cmdline_init): Detect if stdout and/or stderr are connected to the
>>     console.
>>   (svn_cmdline_fputs): Shortcut stdout/stderr to the console when possible.
>>
>> * subversion/libsvn_subr/win32_xlate.c
>>   (includes): Add svn_private_config.h.
>>   (svn_subr__win32_utf8_to_utf16): New function.
>
> Good call!
>
> Can you now please go through all the places (two or three, I think)
> where we're using the internal APR function that does the same thing,
> and use this one instead; then we can finally make the Windows build
> independent of APR sources.

$ grep -r arch/win32 subversion
subversion/bindings/javahl/native/JNIUtil.cpp:#include <arch/win32/apr_arch_utf8.h>
subversion/libsvn_subr/io.c:#include <arch/win32/apr_arch_file_io.h>


Both places use apr_conv_ucs2_to_utf8 and the inverse function, but io.c
also uses some internal APR logic to tell the difference between WInNT
and Win95. I'd vote for simply ripping any pending support for the
ancient non-Unicode Windows OS so that we can get rid of that, too.

-- Brane

-- 
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com

Re: svn commit: r1533994 - in /subversion/trunk/subversion/libsvn_subr: cmdline.c win32_xlate.c win32_xlate.h

Posted by Branko Čibej <br...@wandisco.com>.
On 20.10.2013 23:35, rhuijben@apache.org wrote:
> * subversion/libsvn_subr/cmdline.c
>   (includes): Add conio.h on Windows. Add win32_xlate.
>   (svn_cmdline_init): Detect if stdout and/or stderr are connected to the
>     console.
>   (svn_cmdline_fputs): Shortcut stdout/stderr to the console when possible.
>
> * subversion/libsvn_subr/win32_xlate.c
>   (includes): Add svn_private_config.h.
>   (svn_subr__win32_utf8_to_utf16): New function.

Good call!

Can you now please go through all the places (two or three, I think)
where we're using the internal APR function that does the same thing,
and use this one instead; then we can finally make the Windows build
independent of APR sources.

-- Brane

-- 
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com