This series add a cache for looking up labels while executing a .cmd/.bat file.
Net result is winetest cmd.exe runtime cut down by almost 2/3. This leaves us some space to add some more tests.
Don't expect similar gains for most of command files.
From: Eric Pouech eric.pouech@gmail.com
It's shared by all contexts refererring to the same bat/cmd file.
Signed-off-by: Eric Pouech eric.pouech@gmail.com --- programs/cmd/batch.c | 79 +++++++++++++++++++++++++++-------------- programs/cmd/builtins.c | 2 +- programs/cmd/wcmd.h | 31 +++++++++------- programs/cmd/wcmdmain.c | 6 ++-- 4 files changed, 74 insertions(+), 44 deletions(-)
diff --git a/programs/cmd/batch.c b/programs/cmd/batch.c index 3866831ed68..159b4416a65 100644 --- a/programs/cmd/batch.c +++ b/programs/cmd/batch.c @@ -56,6 +56,53 @@ static RETURN_CODE WCMD_batch_main_loop(void) return return_code; }
+static struct batch_file *find_or_alloc_batch_file(const WCHAR *file) +{ + struct batch_file *batchfile; + struct batch_context *ctx; + + for (ctx = context; ctx; ctx = ctx->prev_context) + { + if (ctx->batch_file && !wcscmp(ctx->batch_file->path_name, file)) + return ctx->batch_file; + } + batchfile = xalloc(sizeof(*batchfile)); + batchfile->ref_count = 0; + batchfile->path_name = xstrdupW(file); + + return batchfile; +} + +static struct batch_context *push_batch_context(WCHAR *command, struct batch_file *batch_file, ULONGLONG pos) +{ + struct batch_context *prev = context; + + context = xalloc(sizeof(struct batch_context)); + context->file_position.QuadPart = pos; + context->command = command; + memset(context->shift_count, 0x00, sizeof(context->shift_count)); + context->prev_context = prev; + context->skip_rest = FALSE; + context->batch_file = batch_file; + batch_file->ref_count++; + + return context; +} + +static struct batch_context *pop_batch_context(struct batch_context *ctx) +{ + struct batch_context *prev = ctx->prev_context; + struct batch_file *batchfile = ctx->batch_file; + if (batchfile && --batchfile->ref_count == 0) + { + free(batchfile->path_name); + free(batchfile); + ctx->batch_file = NULL; + } + free(ctx); + return prev; +} + /**************************************************************************** * WCMD_batch * @@ -74,24 +121,11 @@ static RETURN_CODE WCMD_batch_main_loop(void)
RETURN_CODE WCMD_call_batch(const WCHAR *file, WCHAR *command) { - BATCH_CONTEXT *prev_context; RETURN_CODE return_code = NO_ERROR;
- /* Create a context structure for this batch file. */ - prev_context = context; - context = malloc(sizeof (BATCH_CONTEXT)); - context->file_position.QuadPart = 0; - context->batchfileW = xstrdupW(file); - context->command = command; - memset(context->shift_count, 0x00, sizeof(context->shift_count)); - context->prev_context = prev_context; - context->skip_rest = FALSE; - + context = push_batch_context(command, find_or_alloc_batch_file(file), 0); return_code = WCMD_batch_main_loop(); - - free(context->batchfileW); - free(context); - context = prev_context; + context = pop_batch_context(context);
if (return_code != NO_ERROR && return_code != RETURN_CODE_ABORTED) errorlevel = return_code; @@ -376,7 +410,7 @@ void WCMD_HandleTildeModifiers(WCHAR **start, BOOL atExecute) whereas if you start applying other modifiers to it, you get the filename the batch label is in */ if (*lastModifier == '0' && modifierLen > 1) { - lstrcpyW(outputparam, context->batchfileW); + lstrcpyW(outputparam, context->batch_file->path_name); } else if ((*lastModifier >= '0' && *lastModifier <= '9')) { lstrcpyW(outputparam, WCMD_parameter (context -> command, @@ -634,7 +668,6 @@ RETURN_CODE WCMD_call(WCHAR *command) else if (context) { WCHAR gotoLabel[MAX_PATH]; - BATCH_CONTEXT *prev_context;
lstrcpyW(gotoLabel, param1);
@@ -642,14 +675,7 @@ RETURN_CODE WCMD_call(WCHAR *command) as for loop variables do not survive a call */ WCMD_save_for_loop_context(TRUE);
- prev_context = context; - context = malloc(sizeof (BATCH_CONTEXT)); - context->file_position = prev_context->file_position; /* will be overwritten by WCMD_GOTO below */ - context->batchfileW = prev_context->batchfileW; - context->command = buffer; - memset(context->shift_count, 0x00, sizeof(context->shift_count)); - context->prev_context = prev_context; - context->skip_rest = FALSE; + context = push_batch_context(buffer, context->batch_file, context->file_position.QuadPart);
/* FIXME as commands here can temper with param1 global variable (ugly) */ lstrcpyW(param1, gotoLabel); @@ -657,8 +683,7 @@ RETURN_CODE WCMD_call(WCHAR *command)
WCMD_batch_main_loop();
- free(context); - context = prev_context; + context = pop_batch_context(context); return_code = errorlevel;
/* Restore the for loop context */ diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index c2379598976..21f15c32c4a 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -1752,7 +1752,7 @@ RETURN_CODE WCMD_goto(void) context->skip_rest = TRUE; return RETURN_CODE_ABORTED; } - h = CreateFileW(context->batchfileW, GENERIC_READ, FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, + h = CreateFileW(context->batch_file->path_name, GENERIC_READ, FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); if (h == INVALID_HANDLE_VALUE) { diff --git a/programs/cmd/wcmd.h b/programs/cmd/wcmd.h index 2731743c27a..5138410e8ab 100644 --- a/programs/cmd/wcmd.h +++ b/programs/cmd/wcmd.h @@ -258,23 +258,28 @@ static inline BOOL ends_with_backslash( const WCHAR *path )
int evaluate_if_condition(WCHAR *p, WCHAR **command, int *test, int *negate);
-/* Data structure to hold context when executing batch files */ +/* Data structure to store information about a batch file */ +struct batch_file +{ + unsigned ref_count; /* number of BATCH_CONTEXT attached to this */ + WCHAR *path_name; /* Name of self */ +};
-typedef struct _BATCH_CONTEXT +struct batch_context { - WCHAR *command; /* The command which invoked the batch file */ - LARGE_INTEGER file_position; - WCHAR *batchfileW; /* Name of same */ - int shift_count[10]; /* Offset in terms of shifts for %0 - %9 */ - struct _BATCH_CONTEXT *prev_context; /* Pointer to the previous context block */ - BOOL skip_rest; /* Skip the rest of the batch program and exit */ -} BATCH_CONTEXT; + WCHAR *command; /* The command which invoked the batch file */ + LARGE_INTEGER file_position; + int shift_count[10]; /* Offset in terms of shifts for %0 - %9 */ + struct batch_context *prev_context; /* Pointer to the previous context block */ + BOOL skip_rest; /* Skip the rest of the batch program and exit */ + struct batch_file *batch_file; /* Reference to the file itself */ +};
/* Data structure to handle building lists during recursive calls */
struct env_stack { - BATCH_CONTEXT *context; + struct batch_context *context; struct env_stack *next; union { @@ -324,7 +329,7 @@ void WCMD_set_for_loop_variable(unsigned varidx, const WCHAR *value); */ extern WCHAR quals[MAXSTRING], param1[MAXSTRING], param2[MAXSTRING]; extern int errorlevel; -extern BATCH_CONTEXT *context; +extern struct batch_context *context; extern BOOL delayedsubst;
static inline BOOL WCMD_is_in_context(const WCHAR *ext) @@ -332,9 +337,9 @@ static inline BOOL WCMD_is_in_context(const WCHAR *ext) size_t c_len, e_len; if (!context) return FALSE; if (!ext) return TRUE; - c_len = wcslen(context->batchfileW); + c_len = wcslen(context->batch_file->path_name); e_len = wcslen(ext); - return (c_len > e_len) && !wcsicmp(&context->batchfileW[c_len - e_len], ext); + return (c_len > e_len) && !wcsicmp(&context->batch_file->path_name[c_len - e_len], ext); }
#endif /* !RC_INVOKED */ diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c index 674ea8b1025..fbec86a481a 100644 --- a/programs/cmd/wcmdmain.c +++ b/programs/cmd/wcmdmain.c @@ -53,7 +53,7 @@ typedef struct _SEARCH_CONTEXT extern const WCHAR inbuilt[][10]; extern struct env_stack *pushd_directories;
-BATCH_CONTEXT *context = NULL; +struct batch_context *context = NULL; int errorlevel; WCHAR quals[MAXSTRING], param1[MAXSTRING], param2[MAXSTRING]; BOOL interactive; @@ -3113,7 +3113,7 @@ static WCHAR *fetch_next_line(BOOL feed, BOOL first_line, WCHAR* buffer) if (context) { LARGE_INTEGER zeroli = {.QuadPart = 0}; - HANDLE h = CreateFileW(context->batchfileW, GENERIC_READ, FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, + HANDLE h = CreateFileW(context->batch_file->path_name, GENERIC_READ, FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); if (h == INVALID_HANDLE_VALUE) { @@ -4130,7 +4130,7 @@ RETURN_CODE node_execute(CMD_NODE *node) WCHAR filename[MAX_PATH]; CMD_REDIRECTION *output; HANDLE saved_output; - BATCH_CONTEXT *saved_context = context; + struct batch_context *saved_context = context;
/* pipe LHS & RHS are run outside of any batch context */ context = NULL;
From: Eric Pouech epouech@codeweavers.com
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52720
https://gitlab.winehq.org/wine/wine/-/merge_requests/8200#note_108635
Signed-off-by: Eric Pouech epouech@codeweavers.com --- programs/cmd/batch.c | 144 ++++++++++++++++++++++++++++++++++++++++--- programs/cmd/wcmd.h | 8 +++ 2 files changed, 144 insertions(+), 8 deletions(-)
diff --git a/programs/cmd/batch.c b/programs/cmd/batch.c index 159b4416a65..28ecf917ed3 100644 --- a/programs/cmd/batch.c +++ b/programs/cmd/batch.c @@ -60,6 +60,8 @@ static struct batch_file *find_or_alloc_batch_file(const WCHAR *file) { struct batch_file *batchfile; struct batch_context *ctx; + HANDLE h; + unsigned int i;
for (ctx = context; ctx; ctx = ctx->prev_context) { @@ -70,6 +72,17 @@ static struct batch_file *find_or_alloc_batch_file(const WCHAR *file) batchfile->ref_count = 0; batchfile->path_name = xstrdupW(file);
+ h = CreateFileW(file, GENERIC_READ, FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, + NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + if (h == INVALID_HANDLE_VALUE || !GetFileTime(h, NULL, NULL, &batchfile->last_modified)) + memset(&batchfile->last_modified, 0, sizeof(batchfile->last_modified)); + CloseHandle(h); + + for (i = 0; i < ARRAY_SIZE(batchfile->cache); i++) + { + batchfile->cache[i].label = NULL; + batchfile->cache[i].age = 0; + } return batchfile; }
@@ -95,6 +108,10 @@ static struct batch_context *pop_batch_context(struct batch_context *ctx) struct batch_file *batchfile = ctx->batch_file; if (batchfile && --batchfile->ref_count == 0) { + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(batchfile->cache); i++) + free((void *)batchfile->cache[i].label); free(batchfile->path_name); free(batchfile); ctx->batch_file = NULL; @@ -733,6 +750,89 @@ static BOOL find_next_label(HANDLE h, ULONGLONG end, WCHAR candidate[MAXSTRING], return FALSE; }
+static LARGE_INTEGER li_not_found = {.QuadPart = 0x7fffffffffffffffll}; + +static void insert_label_cache_entry(const WCHAR *label, LARGE_INTEGER from, LARGE_INTEGER at) +{ + struct batch_file *batchfile = context->batch_file; + unsigned int i, worst_index = ~0u, worst_age = 0; + + for (i = 0; i < ARRAY_SIZE(batchfile->cache); i++) + if (batchfile->cache[i].label) + batchfile->cache[i].age++; + else + worst_index = i; + for (i = 0; i < ARRAY_SIZE(batchfile->cache); i++) + { + if (batchfile->cache[i].label && !lstrcmpiW(batchfile->cache[i].label, label) && + batchfile->cache[i].position.QuadPart == at.QuadPart) + { + batchfile->cache[i].age = 0; + /* decrease 'from' position if we have a larger match */ + if (batchfile->cache[i].from.QuadPart > from.QuadPart) + batchfile->cache[i].from.QuadPart = from.QuadPart; + return; + } + } + if (worst_index == ~0u) /* all cache lines are used, find lru */ + { + for (i = 0; i < ARRAY_SIZE(batchfile->cache); i++) + { + if (batchfile->cache[i].age > worst_age) + { + worst_index = i; + worst_age = batchfile->cache[i].age; + } + } + } + free((void*)batchfile->cache[worst_index].label); + batchfile->cache[worst_index].label = xstrdupW(label); + batchfile->cache[worst_index].from = from; + batchfile->cache[worst_index].position = at; + batchfile->cache[worst_index].age = 0; +} + +static BOOL find_label_cache_entry(const WCHAR *label, LARGE_INTEGER from, LARGE_INTEGER *at) +{ + struct batch_file *batchfile = context->batch_file; + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(batchfile->cache); i++) + batchfile->cache[i].age++; + for (i = 0; i < ARRAY_SIZE(batchfile->cache); i++) + { + if (batchfile->cache[i].label && !lstrcmpiW(batchfile->cache[i].label, label) && + batchfile->cache[i].from.QuadPart <= from.QuadPart && + from.QuadPart <= batchfile->cache[i].position.QuadPart) + { + *at = batchfile->cache[i].position; + batchfile->cache[i].age = 0; + return TRUE; + } + } + return FALSE; +} + +static void check_if_valid_label_cache(HANDLE h) +{ + struct batch_file *batchfile = context->batch_file; + FILETIME last; + unsigned int i; + + if (!GetFileTime(h, NULL, NULL, &last) || + batchfile->last_modified.dwHighDateTime != last.dwHighDateTime || + batchfile->last_modified.dwLowDateTime != last.dwLowDateTime) + { + TRACE("Invalidating cache\n"); + batchfile->last_modified = last; + for (i = 0; i < ARRAY_SIZE(batchfile->cache); i++) + { + free((void *)batchfile->cache[i].label); + batchfile->cache[i].label = NULL; + } + } +} + BOOL WCMD_find_label(HANDLE h, const WCHAR *label, LARGE_INTEGER *pos) { LARGE_INTEGER where = *pos, zeroli = {.QuadPart = 0}; @@ -741,20 +841,48 @@ BOOL WCMD_find_label(HANDLE h, const WCHAR *label, LARGE_INTEGER *pos)
if (!*label) return FALSE;
+ check_if_valid_label_cache(h); + if (!SetFilePointerEx(h, *pos, NULL, FILE_BEGIN)) return FALSE; - while (find_next_label(h, ~(ULONGLONG)0, candidate, code_page)) + if (find_label_cache_entry(label, *pos, pos)) { - TRACE("comparing found label %s\n", wine_dbgstr_w(candidate)); - if (!lstrcmpiW(candidate, label)) - return SetFilePointerEx(h, zeroli, pos, FILE_CURRENT); + if (pos->QuadPart != li_not_found.QuadPart) return TRUE; + } + else + { + while (find_next_label(h, ~(ULONGLONG)0, candidate, code_page)) + { + TRACE("comparing found label %s\n", wine_dbgstr_w(candidate)); + if (!lstrcmpiW(candidate, label)) + { + BOOL ret = SetFilePointerEx(h, zeroli, pos, FILE_CURRENT); + if (ret) + insert_label_cache_entry(label, where, *pos); + return ret; + } + } + insert_label_cache_entry(label, where, li_not_found); } TRACE("Label not found, trying from beginning of file\n"); if (!SetFilePointerEx(h, zeroli, NULL, FILE_BEGIN)) return FALSE; - while (find_next_label(h, where.QuadPart, candidate, code_page)) + if (find_label_cache_entry(label, zeroli, pos)) { - TRACE("comparing found label %s\n", wine_dbgstr_w(candidate)); - if (!lstrcmpiW(candidate, label)) - return SetFilePointerEx(h, zeroli, pos, FILE_CURRENT); + if (pos->QuadPart != li_not_found.QuadPart) return TRUE; + } + else + { + while (find_next_label(h, where.QuadPart, candidate, code_page)) + { + TRACE("comparing found label %s\n", wine_dbgstr_w(candidate)); + if (!lstrcmpiW(candidate, label)) + { + BOOL ret = SetFilePointerEx(h, zeroli, pos, FILE_CURRENT); + if (ret) + insert_label_cache_entry(label, zeroli, *pos); + return ret; + } + } + insert_label_cache_entry(label, where, li_not_found); } TRACE("Reached wrap point, label not found\n"); return FALSE; diff --git a/programs/cmd/wcmd.h b/programs/cmd/wcmd.h index 5138410e8ab..f8ea742ee1c 100644 --- a/programs/cmd/wcmd.h +++ b/programs/cmd/wcmd.h @@ -263,6 +263,14 @@ struct batch_file { unsigned ref_count; /* number of BATCH_CONTEXT attached to this */ WCHAR *path_name; /* Name of self */ + FILETIME last_modified; + struct + { + LARGE_INTEGER from; + LARGE_INTEGER position; + unsigned age; + const WCHAR *label; + } cache[8]; };
struct batch_context
From: Eric Pouech eric.pouech@gmail.com
Signed-off-by: Eric Pouech eric.pouech@gmail.com --- programs/cmd/tests/test_builtins.cmd | 111 +++++++++++++++++++++++ programs/cmd/tests/test_builtins.cmd.exp | 11 +++ 2 files changed, 122 insertions(+)
diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd index 8ae733681eb..7db955f634d 100644 --- a/programs/cmd/tests/test_builtins.cmd +++ b/programs/cmd/tests/test_builtins.cmd @@ -4180,6 +4180,117 @@ start /wait cmd /cutf8.cmd if errorlevel 1 (echo Failure) else echo Success del utf8.cmd
+echo ------------ Testing alteration while executing ------ +rem In all the tests, the offsets (esp. for labels) must remain the same +rem Calling a non existing label will generate a message on stderr but nothing on stdout + +rem overwrite label before current position +echo @echo off > run.cmd +echo goto doit >> run.cmd +echo :labelAA >> run.cmd +echo echo AA >> run.cmd +echo goto :eof >> run.cmd +echo :doit >> run.cmd +echo copy /Y ovr.cmd run.cmd ^> NUL >> run.cmd +echo call :labelAA >> run.cmd +echo call :labelBB >> run.cmd + +echo @echo off > ovr.cmd +echo goto doit >> ovr.cmd +echo :labelBB >> ovr.cmd +echo echo BB >> ovr.cmd +echo goto :eof >> ovr.cmd +echo :doit >> ovr.cmd +echo copy /Y ovr.cmd run.cmd ^> NUL >> ovr.cmd +echo call :labelAA >> ovr.cmd +echo call :labelBB >> ovr.cmd + +call run.cmd + +rem overwrite label after current position +echo --- +echo @echo off > run.cmd +echo copy /Y ovr.cmd run.cmd ^> NUL >> run.cmd +echo call :labelAA >> run.cmd +echo call :labelBB >> run.cmd +echo goto :eof >> run.cmd +echo :labelAA >> run.cmd +echo echo AA >> run.cmd +echo goto :eof >> run.cmd + +echo @echo off > ovr.cmd +echo copy /Y ovr.cmd run.cmd ^> NUL >> ovr.cmd +echo call :labelAA >> ovr.cmd +echo call :labelBB >> ovr.cmd +echo goto :eof >> ovr.cmd +echo :labelBB >> ovr.cmd +echo echo BB >> ovr.cmd +echo goto :eof >> ovr.cmd + +call run.cmd + +rem overwrite label before current position with another label with same name +echo --- +echo @echo off > run.cmd +echo goto doit >> run.cmd +echo :labelAA >> run.cmd +echo echo A1 >> run.cmd +echo goto :eof >> run.cmd +echo :labelAA >> run.cmd +echo echo A2 >> run.cmd +echo goto :eof >> run.cmd +echo :doit >> run.cmd +echo copy /Y ovr.cmd run.cmd ^> NUL >> run.cmd +echo call :labelAA >> run.cmd +echo call :labelBB >> run.cmd + +echo @echo off > ovr.cmd +echo goto doit >> ovr.cmd +echo :labelBB >> ovr.cmd +echo echo BB >> ovr.cmd +echo goto :eof >> ovr.cmd +echo :labelAA >> ovr.cmd +echo echo A2 >> ovr.cmd +echo goto :eof >> ovr.cmd +echo :doit >> ovr.cmd +echo copy /Y ovr.cmd run.cmd ^> NUL >> ovr.cmd +echo call :labelAA >> ovr.cmd +echo call :labelBB >> ovr.cmd + +call run.cmd + +rem overwrite label after current position with another label with same name +echo --- +echo @echo off > run.cmd +echo copy /Y ovr.cmd run.cmd ^> NUL >> run.cmd +echo call :labelAA >> run.cmd +echo call :labelBB >> run.cmd +echo goto :eof >> run.cmd +echo :labelAA >> run.cmd +echo echo A1 >> run.cmd +echo goto :eof >> run.cmd +echo :labelAA >> run.cmd +echo echo A2 >> run.cmd +echo goto :eof >> run.cmd + +echo @echo off > ovr.cmd +echo copy /Y ovr.cmd ovr.cmd ^> NUL >> ovr.cmd +echo call :labelAA >> ovr.cmd +echo call :labelBB >> ovr.cmd +echo goto :eof >> ovr.cmd +echo :labelBB >> ovr.cmd +echo echo BB >> ovr.cmd +echo goto :eof >> ovr.cmd +echo :labelAA >> ovr.cmd +echo echo A2 >> ovr.cmd +echo goto :eof >> ovr.cmd + +call run.cmd + +rem cleanup +echo --- +delete run.cmd +delete ovr.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 95feb67602b..1d31b19e01c 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -2166,6 +2166,17 @@ Success ---- Testing nasty bits ---- ------------ Testing updated code page execution ------------ Success +------------ Testing alteration while executing ------ +BB@space@ +--- +BB@space@ +--- +A2@space@ +BB@space@ +--- +A2@space@ +BB@space@ +--- ------------ Testing combined CALLs/GOTOs ------------ world cheball
-
Eric Pouech -
eric pouech (＠epo)