[PATCH v2 0/2] MR6885: cmd: Use the console output codepage to read batch files.
To prevent performance degradation, we will cache the result of GetConsoleOutputCP() after executing every external command. -- v2: cmd: Use the console output codepage to read batch files. cmd/tests: Add updated code page test in batch file. https://gitlab.winehq.org/wine/wine/-/merge_requests/6885
From: Akihiro Sagawa <sagawa.aki(a)gmail.com> --- programs/cmd/tests/batch.c | 31 ++++++++++++++++++++++++ programs/cmd/tests/test_builtins.cmd | 9 +++++++ programs/cmd/tests/test_builtins.cmd.exp | 2 ++ 3 files changed, 42 insertions(+) diff --git a/programs/cmd/tests/batch.c b/programs/cmd/tests/batch.c index 6bc5e916ea9..03a05f34215 100644 --- a/programs/cmd/tests/batch.c +++ b/programs/cmd/tests/batch.c @@ -31,11 +31,38 @@ static DWORD path_len; static char shortpath[MAX_PATH]; static DWORD shortpath_len; +static BOOL parse_hexadecimal(const char *p, char *dest) +{ + unsigned char c; + if (*p++ != '@') return FALSE; + if (*p++ != '\\') return FALSE; + if (*p++ != 'x') return FALSE; + + if (*p >= '0' && *p <= '9') c = *p - '0'; + else if (*p >= 'a' && *p <= 'f') c = *p - 'a' + 10; + else if (*p >= 'A' && *p <= 'F') c = *p - 'A' + 10; + else return FALSE; + p++; + + c <<= 4; + + if (*p >= '0' && *p <= '9') c += *p - '0'; + else if (*p >= 'a' && *p <= 'f') c += *p - 'a' + 10; + else if (*p >= 'A' && *p <= 'F') c += *p - 'A' + 10; + else return FALSE; + p++; + + if (*p != '@') return FALSE; + *dest = (char)c; + return TRUE; +} + /* Convert to DOS line endings, and substitute escaped whitespace chars with real ones */ static const char* convert_input_data(const char *data, DWORD size, DWORD *new_size) { static const char escaped_space[] = {'@','s','p','a','c','e','@'}; static const char escaped_tab[] = {'@','t','a','b','@'}; + static const char escaped_hexadecimal[] = {'@','\\','x','.','.','@'}; DWORD i, eol_count = 0; char *ptr, *new_data; @@ -60,6 +87,10 @@ static const char* convert_input_data(const char *data, DWORD size, DWORD *new_s && !memcmp(data + i, escaped_tab, sizeof(escaped_tab))) { *ptr++ = '\t'; i += sizeof(escaped_tab) - 1; + } else if (data + i + sizeof(escaped_hexadecimal) - 1 < data + size + && parse_hexadecimal(data + i, ptr)) { + ptr++; + i += sizeof(escaped_hexadecimal) - 1; } else { *ptr++ = data[i]; } diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 2d37bdc4c59..907b8d123f2 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -4130,6 +4130,15 @@ echo echo shouldnot >> foobar.bat call foobar.bat if exist foobar.bat (echo stillthere & erase /q foobar.bat >NUL) +echo ------------ Testing updated code page execution ------------ +echo @echo off>utf8.cmd +echo chcp 65001>>utf8.cmd +echo set utf8=@\xE3@@\xA1@@\xA1@@\xE3@@\xA1@@\xA1@>>utf8.cmd +echo if not "%%utf8:~0,2%%"=="%%utf8%%" exit 1 >>utf8.cmd +start /wait cmd /cutf8.cmd +if not errorlevel 1 echo Success +del utf8.cmd + echo ------------ Testing combined CALLs/GOTOs ------------ echo @echo off>foo.cmd echo goto :eof>>foot.cmd diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 6137c594359..46d045f3eb0 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -2122,6 +2122,8 @@ Normal+tab+garbage Success @todo_wine(a)Success ---- Testing nasty bits ---- +------------ Testing updated code page execution ------------ +(a)todo_wine@Success ------------ Testing combined CALLs/GOTOs ------------ world cheball -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6885
From: Akihiro Sagawa <sagawa.aki(a)gmail.com> To prevent performance degradation, we will cache the result of GetConsoleOutputCP() after executing every external command. --- programs/cmd/batch.c | 20 ++++++++++++++++---- programs/cmd/builtins.c | 1 + programs/cmd/tests/test_builtins.cmd.exp | 2 +- programs/cmd/wcmd.h | 1 + programs/cmd/wcmdmain.c | 6 ++++++ 5 files changed, 25 insertions(+), 5 deletions(-) diff --git a/programs/cmd/batch.c b/programs/cmd/batch.c index 20ae894dbfd..39ce96656f9 100644 --- a/programs/cmd/batch.c +++ b/programs/cmd/batch.c @@ -24,6 +24,8 @@ WINE_DEFAULT_DEBUG_CHANNEL(cmd); +static UINT file_code_page; /* code page for reading batch file */ + static RETURN_CODE WCMD_batch_main_loop(void) { RETURN_CODE return_code = NO_ERROR; @@ -205,6 +207,18 @@ WCHAR *WCMD_parameter (WCHAR *s, int n, WCHAR **start, BOOL raw, return WCMD_parameter_with_delims (s, n, start, raw, wholecmdline, L" \t,=;"); } +/**************************************************************************** + * WCMD_update_file_code_page + * + * Update the file code page used in the WCMD_fgets(). + * Since the GetConsoleOutputCP() is slow, we cache the result in advance. + */ +void WCMD_update_file_code_page(void) +{ + UINT cp = GetConsoleOutputCP(); + file_code_page = cp ? cp : CP_OEMCP; +} + /**************************************************************************** * WCMD_fgets * @@ -239,10 +253,8 @@ WCHAR *WCMD_fgets(WCHAR *buf, DWORD noChars, HANDLE h) else { LARGE_INTEGER filepos; char *bufA; - UINT cp; const char *p; - cp = GetOEMCP(); bufA = xalloc(noChars); /* Save current file position */ @@ -256,7 +268,7 @@ WCHAR *WCMD_fgets(WCHAR *buf, DWORD noChars, HANDLE h) } /* Find first EOL */ - for (p = bufA; p < (bufA + charsRead); p = CharNextExA(cp, p, 0)) { + for (p = bufA; p < (bufA + charsRead); p = CharNextExA(file_code_page, p, 0)) { if (*p == '\n' || *p == '\r') break; } @@ -265,7 +277,7 @@ WCHAR *WCMD_fgets(WCHAR *buf, DWORD noChars, HANDLE h) filepos.QuadPart += p - bufA + 1 + (*p == '\r' ? 1 : 0); SetFilePointerEx(h, filepos, NULL, FILE_BEGIN); - i = MultiByteToWideChar(cp, 0, bufA, p - bufA, buf, noChars); + i = MultiByteToWideChar(file_code_page, 0, bufA, p - bufA, buf, noChars); free(bufA); } diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index b78ae9d293b..151c71c90ab 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -3382,6 +3382,7 @@ RETURN_CODE WCMD_start(WCHAR *args) WaitForSingleObject( pi.hProcess, INFINITE ); GetExitCodeProcess( pi.hProcess, &exit_code ); errorlevel = (exit_code == STILL_ACTIVE) ? NO_ERROR : exit_code; + WCMD_update_file_code_page(); CloseHandle(pi.hProcess); CloseHandle(pi.hThread); } diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index 46d045f3eb0..ff6f944cffb 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -2123,7 +2123,7 @@ Success @todo_wine(a)Success ---- Testing nasty bits ---- ------------ Testing updated code page execution ------------ -(a)todo_wine@Success +Success ------------ Testing combined CALLs/GOTOs ------------ world cheball diff --git a/programs/cmd/wcmd.h b/programs/cmd/wcmd.h index d4adc7fd7c8..20293221e38 100644 --- a/programs/cmd/wcmd.h +++ b/programs/cmd/wcmd.h @@ -199,6 +199,7 @@ RETURN_CODE WCMD_volume(void); RETURN_CODE WCMD_mklink(WCHAR *args); RETURN_CODE WCMD_change_drive(WCHAR drive); +void WCMD_update_file_code_page (void); WCHAR *WCMD_fgets (WCHAR *buf, DWORD n, HANDLE stream); WCHAR *WCMD_parameter (WCHAR *s, int n, WCHAR **start, BOOL raw, BOOL wholecmdline); WCHAR *WCMD_parameter_with_delims (WCHAR *s, int n, WCHAR **start, BOOL raw, diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c index 84d3f52970e..39b865ae528 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -1435,7 +1435,10 @@ static RETURN_CODE run_full_path(const WCHAR *file, WCHAR *full_cmdline, BOOL ca } if (!interactive || (console && !HIWORD(console))) + { WaitForSingleObject(handle, INFINITE); + WCMD_update_file_code_page(); + } GetExitCodeProcess(handle, &exit_code); errorlevel = (exit_code == STILL_ACTIVE) ? NO_ERROR : exit_code; @@ -3871,6 +3874,9 @@ int __cdecl wmain (int argc, WCHAR *argvW[]) */ control_c_event = CreateEventW(NULL, TRUE, FALSE, NULL); + /* Initialize the batch file code page */ + WCMD_update_file_code_page(); + /* Can't use argc/argv as it will have stripped quotes from parameters * meaning cmd.exe /C echo "quoted string" is impossible */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6885
Hi, @epo. I have updated the test to follow your suggestions. Unfortunately, job 149935 was lost. So I rewrote the hexadecimal parser with my own implementation. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6885#note_101394
thanks for updating the MR overall looks good to me but I don't like having a global variable holding the current code page: * means that we need to keep synchronisation of cache wrt. real status * note that a background app could change console code page, so changing code page upon external process execution covers most of the aspects (eg invokind 'chcp'), but not all of them the reason for slowdown (even timeout in running the tests), comes from the label finding routines that have to rescan potentially the whole batch file, hence calling a zillions on time WCMD_fgets() (almost 3 millions calls when running the non regression tests) calling once GetConsoleOutputCP() before each label search (instead of once call for every line in every search) solves the speed issue see attached mbox for an example would you mind using this instead? [confactor.patch](/uploads/3cac3d52c288797edbe080f36950774e/confactor.patch) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6885#note_101473
participants (3)
-
Akihiro Sagawa -
Akihiro Sagawa (＠sgwaki)
-
eric pouech (＠epo)