The following series drastically improves cmd performance by caching label resolution when executing a batch file.
Testbot is hitting a timeout when running the tests. See https://bugs.winehq.org/show_bug.cgi?id=52720
Local tests show a reduction of test duration of 90%! Don't always expect those numbers; in some extreme cases, the patchset will even slightly increase execution time.
Note: the reason why testbot started timing out is still unknown.
Signed-off-by: Eric Pouech eric.pouech@gmail.com ---
Eric Pouech (2): programs/cmd: introduce structure BATCH_FILE to hold information about a .cmd file programs/cmd: add label => file offset lookup in BATCH_FILE
programs/cmd/batch.c | 153 ++++++++++++++++++++++++++++++++++++++-- programs/cmd/builtins.c | 63 +++-------------- programs/cmd/wcmd.h | 19 ++++- 3 files changed, 172 insertions(+), 63 deletions(-)
keeping BATCH_CONTEXT as the execution context of a BATCH_FILE
Signed-off-by: Eric Pouech eric.pouech@gmail.com
--- programs/cmd/batch.c | 48 ++++++++++++++++++++++++++++++++++++++++++------ programs/cmd/wcmd.h | 10 ++++++++-- 2 files changed, 50 insertions(+), 8 deletions(-)
diff --git a/programs/cmd/batch.c b/programs/cmd/batch.c index 9a262c5fec5..ab9b90e48ca 100644 --- a/programs/cmd/batch.c +++ b/programs/cmd/batch.c @@ -26,6 +26,40 @@ extern struct env_stack *saved_environment;
WINE_DEFAULT_DEBUG_CHANNEL(cmd);
+static BATCH_FILE *linkto_batch_file( const WCHAR *file, HANDLE h ) +{ + BATCH_FILE *batchfile; + BATCH_CONTEXT *ctx; + FILETIME last; + + GetFileTime(h, NULL, NULL, &last); + for (ctx = context; ctx; ctx = ctx->prev_context) + { + if (ctx->file && !wcscmp( ctx->file->batchfileW, file ) && + !CompareFileTime( &last, &ctx->file->last_modified )) + { + ctx->file->ref_count++; + return ctx->file; + } + } + batchfile = malloc( sizeof(*batchfile) ); + batchfile->ref_count = 1; + batchfile->batchfileW = heap_strdupW( file ); + batchfile->last_modified = last; + + return batchfile; +} + +static void unref_batch_file( BATCH_CONTEXT *ctx ) +{ + BATCH_FILE *batchfile = ctx->file; + if (--batchfile->ref_count == 0) { + heap_free( batchfile->batchfileW ); + free( batchfile ); + ctx->file = NULL; + } +} + /**************************************************************************** * WCMD_batch * @@ -46,6 +80,7 @@ void WCMD_batch (WCHAR *file, WCHAR *command, BOOL called, WCHAR *startLabel, HA { HANDLE h = INVALID_HANDLE_VALUE; BATCH_CONTEXT *prev_context; + BATCH_FILE *batchfile;
if (startLabel == NULL) { h = CreateFileW (file, GENERIC_READ, FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, @@ -65,14 +100,15 @@ void WCMD_batch (WCHAR *file, WCHAR *command, BOOL called, WCHAR *startLabel, HA * Create a context structure for this batch file. */
+ batchfile = linkto_batch_file( file, h ); prev_context = context; - context = LocalAlloc (LMEM_FIXED, sizeof (BATCH_CONTEXT)); + context = malloc(sizeof(BATCH_CONTEXT)); context -> h = h; - context->batchfileW = heap_strdupW(file); context -> command = command; memset(context -> shift_count, 0x00, sizeof(context -> shift_count)); context -> prev_context = prev_context; context -> skip_rest = FALSE; + context -> file = batchfile;
/* If processing a call :label, 'goto' the label in question */ if (startLabel) { @@ -110,8 +146,8 @@ void WCMD_batch (WCHAR *file, WCHAR *command, BOOL called, WCHAR *startLabel, HA * to the caller's caller. */
- heap_free(context->batchfileW); - LocalFree (context); + unref_batch_file( context ); + free(context); if ((prev_context != NULL) && (!called)) { WINE_TRACE("Batch completed, but was not 'called' so skipping outer batch too\n"); prev_context -> skip_rest = TRUE; @@ -417,7 +453,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->file->batchfileW); } else if ((*lastModifier >= '0' && *lastModifier <= '9')) { lstrcpyW(outputparam, WCMD_parameter (context -> command, @@ -675,7 +711,7 @@ void WCMD_call (WCHAR *command) { li.QuadPart = 0; li.u.LowPart = SetFilePointer(context -> h, li.u.LowPart, &li.u.HighPart, FILE_CURRENT); - WCMD_batch (context->batchfileW, command, TRUE, gotoLabel, context->h); + WCMD_batch (context->file->batchfileW, command, TRUE, gotoLabel, context->h); SetFilePointer(context -> h, li.u.LowPart, &li.u.HighPart, FILE_BEGIN);
diff --git a/programs/cmd/wcmd.h b/programs/cmd/wcmd.h index 234c253b49a..ebce5359eb8 100644 --- a/programs/cmd/wcmd.h +++ b/programs/cmd/wcmd.h @@ -149,16 +149,22 @@ 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 */ +typedef struct _BATCH_FILE { + unsigned ref_count; /* number of BATCH_CONTEXT attached to this */ + WCHAR *batchfileW; /* Name of self */ + FILETIME last_modified;/* last modified date of the file */ +} BATCH_FILE;
+/* Data structure to hold context when executing batch files */ typedef struct _BATCH_CONTEXT { WCHAR *command; /* The command which invoked the batch file */ HANDLE h; /* Handle to the open batch file */ - 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 */ CMD_LIST *toExecute; /* Commands left to be executed */ + BATCH_FILE *file; /* Reference to the file itself */ } BATCH_CONTEXT;
/* Data structure to handle building lists during recursive calls */
The whole .cmd file was reparsed every time a label was used (hence every time {goto, call to a local label} was invoked inside the file).
This patch generates for each .cmd file a table with label => file position to speed up the process.
Native cmd supports modification of batch file even when executing it, so the cache is invalidated when the batch file gets modified.
Locally, 'make test' went from 55s down to 6s. Marvin was hitting a 120s timeout; it no longer times out, and runs in ~20s. YMMV.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52720 Signed-off-by: Eric Pouech eric.pouech@gmail.com
--- programs/cmd/batch.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++ programs/cmd/builtins.c | 63 ++++------------------------ programs/cmd/wcmd.h | 9 ++++ 3 files changed, 122 insertions(+), 55 deletions(-)
diff --git a/programs/cmd/batch.c b/programs/cmd/batch.c index ab9b90e48ca..52813966686 100644 --- a/programs/cmd/batch.c +++ b/programs/cmd/batch.c @@ -46,6 +46,8 @@ static BATCH_FILE *linkto_batch_file( const WCHAR *file, HANDLE h ) batchfile->ref_count = 1; batchfile->batchfileW = heap_strdupW( file ); batchfile->last_modified = last; + batchfile->labels_cache = NULL; + batchfile->num_labels = -1;
return batchfile; } @@ -54,12 +56,31 @@ static void unref_batch_file( BATCH_CONTEXT *ctx ) { BATCH_FILE *batchfile = ctx->file; if (--batchfile->ref_count == 0) { + int i; heap_free( batchfile->batchfileW ); + for (i = 0; i < batchfile->num_labels; i++) + heap_free( (void*)batchfile->labels_cache[i].label ); + free( batchfile->labels_cache ); free( batchfile ); ctx->file = NULL; } }
+/* check if batch file has been modified... if so, link to the updated instance + * (as cached information need to be recomputed) + */ +static void update_batch_file( BATCH_CONTEXT *ctx ) +{ + FILETIME last; + GetFileTime( ctx->h, NULL, NULL, &last ); + if (CompareFileTime( &last, &ctx->file->last_modified ) != 0) + { + BATCH_FILE *new = linkto_batch_file( ctx->file->batchfileW, &last ); + unref_batch_file( ctx ); + ctx->file = new; + } +} + /**************************************************************************** * WCMD_batch * @@ -722,3 +743,87 @@ void WCMD_call (WCHAR *command) { } } } + +static void WCMD_build_label_offsets( BATCH_CONTEXT *ctx ) +{ + WCHAR string[MAX_PATH]; + unsigned count = 0; + const WCHAR labelEndsW[] = L"><|& :\t"; + BATCH_FILE *batchfile; + LABEL_REFERENCE *lblref; + LARGE_INTEGER li; + + batchfile = ctx->file; + batchfile->num_labels = 0; + li.QuadPart = 0; + SetFilePointer(ctx->h, li.u.LowPart, &li.u.HighPart, FILE_BEGIN); + + while (WCMD_fgets( string, ARRAY_SIZE(string), ctx->h )) { + WCHAR *str = string; + WCHAR *labelend; + + /* Ignore leading whitespace or no-echo character */ + while (*str=='@' || iswspace (*str)) str++; + + /* If the first real character is a : then this is a label */ + if (*str == ':') { + str++; + + /* Skip spaces between : and label */ + while (iswspace (*str)) str++; + WINE_TRACE("str before brk %s\n", wine_dbgstr_w(str)); + + /* Label ends at whitespace or redirection characters */ + labelend = wcspbrk( str, labelEndsW ); + if (labelend) *labelend = 0x00; + + if (batchfile->num_labels + 1 >= count) { + void *new = realloc( batchfile->labels_cache, + (count = count ? count * 2 : 16) * sizeof(*batchfile->labels_cache) ); + if (!new) + return; + batchfile->labels_cache = new; + } + lblref = &batchfile->labels_cache[batchfile->num_labels]; + + lblref->label = heap_strdupW(str); + lblref->file_offset.QuadPart = 0; + lblref->file_offset.u.LowPart = SetFilePointer( ctx->h, lblref->file_offset.u.LowPart, + &lblref->file_offset.u.HighPart, FILE_CURRENT ); + WINE_TRACE( "\tadding %ls at %#I64x\n", + lblref->label, lblref->file_offset.QuadPart ); + batchfile->num_labels++; + } + } +} + +LABEL_REFERENCE* WCMD_find_label_reference( BATCH_CONTEXT* ctx, const WCHAR *label ) +{ + BATCH_FILE *batchfile; + LARGE_INTEGER startli; + int start, i; + + if (!*label) return NULL; + + startli.QuadPart = 0; + startli.u.LowPart = SetFilePointer( ctx->h, startli.u.LowPart, &startli.u.HighPart, FILE_CURRENT ); + + update_batch_file( ctx ); + batchfile = ctx->file; + if (batchfile->num_labels == -1) { + WCMD_build_label_offsets( ctx ); + SetFilePointer( ctx->h, startli.u.LowPart, &startli.u.HighPart, FILE_BEGIN ); + } + + for (start = 0; start < batchfile->num_labels; start++) + if (batchfile->labels_cache[start].file_offset.QuadPart >= startli.QuadPart) break; + /* search from current file position until end of file... */ + for (i = start; i < batchfile->num_labels; i++) + if (!wcscmp( batchfile->labels_cache[i].label, label )) + return &batchfile->labels_cache[i]; + /* if not found, restart from beg of file */ + for (i = 0; i < start; i++) + if (!wcscmp( batchfile->labels_cache[i].label, label )) + return &batchfile->labels_cache[i]; + return NULL; +} diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index 963a9eaf361..509c89cb565 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -2549,7 +2549,6 @@ void WCMD_give_help (const WCHAR *args)
void WCMD_goto (CMD_LIST **cmdList) {
- WCHAR string[MAX_PATH]; WCHAR *labelend = NULL; const WCHAR labelEndsW[] = L"><|& :\t";
@@ -2557,7 +2556,8 @@ void WCMD_goto (CMD_LIST **cmdList) { if (cmdList) *cmdList = NULL;
if (context != NULL) { - WCHAR *paramStart = param1, *str; + WCHAR *paramStart = param1; + LABEL_REFERENCE *lblref;
if (param1[0] == 0x00) { WCMD_output_stderr(WCMD_LoadMessage(WCMD_NOARG)); @@ -2576,59 +2576,12 @@ void WCMD_goto (CMD_LIST **cmdList) { if (labelend) *labelend = 0x00; WINE_TRACE("goto label: '%s'\n", wine_dbgstr_w(paramStart));
- /* Loop through potentially twice - once from current file position - through to the end, and second time from start to current file - position */ - if (*paramStart) { - int loop; - LARGE_INTEGER startli; - for (loop=0; loop<2; loop++) { - if (loop==0) { - /* On first loop, save the file size */ - startli.QuadPart = 0; - startli.u.LowPart = SetFilePointer(context -> h, startli.u.LowPart, - &startli.u.HighPart, FILE_CURRENT); - } else { - /* On second loop, start at the beginning of the file */ - WINE_TRACE("Label not found, trying from beginning of file\n"); - if (loop==1) SetFilePointer (context -> h, 0, NULL, FILE_BEGIN); - } - - while (WCMD_fgets (string, ARRAY_SIZE(string), context -> h)) { - str = string; - - /* Ignore leading whitespace or no-echo character */ - while (*str=='@' || iswspace (*str)) str++; - - /* If the first real character is a : then this is a label */ - if (*str == ':') { - str++; - - /* Skip spaces between : and label */ - while (iswspace (*str)) str++; - WINE_TRACE("str before brk %s\n", wine_dbgstr_w(str)); - - /* Label ends at whitespace or redirection characters */ - labelend = wcspbrk(str, labelEndsW); - if (labelend) *labelend = 0x00; - WINE_TRACE("comparing found label %s\n", wine_dbgstr_w(str)); - - if (lstrcmpiW (str, paramStart) == 0) return; - } - - /* See if we have gone beyond the end point if second time through */ - if (loop==1) { - LARGE_INTEGER curli; - curli.QuadPart = 0; - curli.u.LowPart = SetFilePointer(context -> h, curli.u.LowPart, - &curli.u.HighPart, FILE_CURRENT); - if (curli.QuadPart > startli.QuadPart) { - WINE_TRACE("Reached wrap point, label not found\n"); - break; - } - } - } - } + lblref = WCMD_find_label_reference( context, paramStart ); + if (lblref) { + LARGE_INTEGER curli; + curli.QuadPart = lblref->file_offset.QuadPart; + SetFilePointer(context -> h, curli.u.LowPart, &curli.u.HighPart, FILE_BEGIN); + return; }
WCMD_output_stderr(WCMD_LoadMessage(WCMD_NOTARGET)); diff --git a/programs/cmd/wcmd.h b/programs/cmd/wcmd.h index ebce5359eb8..0eaf3e94d7c 100644 --- a/programs/cmd/wcmd.h +++ b/programs/cmd/wcmd.h @@ -149,11 +149,18 @@ static inline BOOL ends_with_backslash( const WCHAR *path )
int evaluate_if_condition(WCHAR *p, WCHAR **command, int *test, int *negate);
+typedef struct _LABEL_REFERENCE { + const WCHAR* label; + LARGE_INTEGER file_offset; +} LABEL_REFERENCE; + /* Data structure to store information about a batch file */ typedef struct _BATCH_FILE { unsigned ref_count; /* number of BATCH_CONTEXT attached to this */ WCHAR *batchfileW; /* Name of self */ FILETIME last_modified;/* last modified date of the file */ + LABEL_REFERENCE *labels_cache; + int num_labels; /* -1 when not loaded */ } BATCH_FILE;
/* Data structure to hold context when executing batch files */ @@ -167,6 +174,8 @@ typedef struct _BATCH_CONTEXT { BATCH_FILE *file; /* Reference to the file itself */ } BATCH_CONTEXT;
+LABEL_REFERENCE* WCMD_find_label_reference( BATCH_CONTEXT*, const WCHAR * ); + /* Data structure to handle building lists during recursive calls */
struct env_stack
-
Eric Pouech