78471: Subject: [2/2] cmd: Avoid rereading batch file for every call/goto executed (try 2)

buildbot at kegel.com buildbot at kegel.com
Wed Sep 7 04:22:18 CDT 2011


This is an experimental automated build and test service.
Please feel free to ignore this email while we work the kinks out.

The Buildbot has detected a failed build on builder runtests-default while building Wine.
Full details are available at: http://buildbot.kegel.com/builders/runtests-default/builds/38 (though maybe not for long, as I'm still reinstalling the buildbot periodically while experimenting)
BUILD FAILED: failed shell_3


For more info about this message, see http://wiki.winehq.org/BuildBot


-------------- next part --------------
From: Frédéric Delanoy <frederic.delanoy at gmail.com>
Subject: [1/2] cmd: Use correct type instead of void* for prev_context field of BATCH_CONTEXT struct
Message-Id: <1315386054-15445-1-git-send-email-frederic.delanoy at gmail.com>
Date: Wed,  7 Sep 2011 11:00:53 +0200

---
 programs/cmd/wcmd.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/programs/cmd/wcmd.h b/programs/cmd/wcmd.h
index c4ece13..bd07f5e 100644
--- a/programs/cmd/wcmd.h
+++ b/programs/cmd/wcmd.h
@@ -120,12 +120,12 @@ void      WCMD_execute (const WCHAR *orig_command, const WCHAR *redirects,
 
 /* Data structure to hold context when executing batch files */
 
-typedef struct {
+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 */
-  void *prev_context;	/* Pointer to the previous context block */
+  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_CONTEXT;
-- 
1.7.6

From: Frédéric Delanoy <frederic.delanoy at gmail.com>
Subject: [2/2] cmd: Avoid rereading batch file for every call/goto executed (try 2)
Message-Id: <1315386054-15445-2-git-send-email-frederic.delanoy at gmail.com>
Date: Wed,  7 Sep 2011 11:00:54 +0200

Added a labels cache.
Current code was parsing the whole file from start for every call/goto, making
it terribly slow to run the test suite.

try 2: store labels cache in batch context
---
 programs/cmd/batch.c    |   12 ++++
 programs/cmd/builtins.c |  135 +++++++++++++++++++++++++++++++++++++++--------
 programs/cmd/wcmd.h     |   10 ++++
 3 files changed, 135 insertions(+), 22 deletions(-)

diff --git a/programs/cmd/batch.c b/programs/cmd/batch.c
index f010f28..16aa194 100644
--- a/programs/cmd/batch.c
+++ b/programs/cmd/batch.c
@@ -49,6 +49,7 @@ void WCMD_batch (WCHAR *file, WCHAR *command, int called, WCHAR *startLabel, HAN
 
   HANDLE h = INVALID_HANDLE_VALUE;
   BATCH_CONTEXT *prev_context;
+  SIZE_T i;
 
   if (startLabel == NULL) {
     h = CreateFileW (file, GENERIC_READ, FILE_SHARE_READ,
@@ -74,6 +75,9 @@ void WCMD_batch (WCHAR *file, WCHAR *command, int called, WCHAR *startLabel, HAN
   context->batchfileW = WCMD_strdupW(file);
   context -> command = command;
   memset(context -> shift_count, 0x00, sizeof(context -> shift_count));
+  (context->labels).names = 0x00;
+  (context->labels).positions = 0x00;
+  (context->labels).count = 0;
   context -> prev_context = prev_context;
   context -> skip_rest = FALSE;
 
@@ -104,6 +108,14 @@ void WCMD_batch (WCHAR *file, WCHAR *command, int called, WCHAR *startLabel, HAN
  */
 
   HeapFree(GetProcessHeap(), 0, context->batchfileW);
+
+  if ((context->labels).names) {
+    for (i=0; i < (context->labels).count; i++) {
+        HeapFree(GetProcessHeap(), 0, (context->labels).names[i]);
+    }
+    HeapFree(GetProcessHeap(), 0, (context->labels).names);
+    HeapFree(GetProcessHeap(), 0, (context->labels).positions);
+  }
   LocalFree (context);
   if ((prev_context != NULL) && (!called)) {
     prev_context -> skip_rest = TRUE;
diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c
index 7063eb7..f9fdba8 100644
--- a/programs/cmd/builtins.c
+++ b/programs/cmd/builtins.c
@@ -1342,10 +1342,113 @@ void WCMD_give_help (const WCHAR *command) {
   return;
 }
 
+/*******************************************************************
+ * get_label_position
+ *
+ * Retrieves the position of label in the current batch file.
+ *
+ * PARAMS
+ *  label [I] input label, non NULL or "eof"
+ *  pos   [O] pointer to the file position of label, if found
+ *
+ * RETURNS
+ *  Success: Returns TRUE.
+ *           *pos points to the file position of label, relative to the
+ *           beginning of the file.
+ *  Failure: Returns FALSE.
+ */
+static BOOL get_label_position(const WCHAR *label, LARGE_INTEGER *pos) {
+
+    WCHAR **keys;
+    LARGE_INTEGER *vals;
+    SIZE_T elems_count, i;
+    BATCH_CONTEXT *cache_context;
+    BOOL labels_cache_initialised;
+
+    /*
+     * Locate nearest cache, i.e. of first non-null context ancestor (or self)
+     * (a "non-label" context)
+     */
+    cache_context = context;
+    while (cache_context && (*cache_context->command) == ':')
+        cache_context = cache_context->prev_context;
+
+    labels_cache_initialised = (cache_context->labels.names != NULL);
+    if (!labels_cache_initialised) {
+        WCHAR *key;
+        LARGE_INTEGER val;
+
+        SIZE_T capacity = 8;
+        WCHAR string[MAX_PATH];
+        WCHAR *str;
+
+        elems_count = 0;
+        keys = HeapAlloc(GetProcessHeap(), 0, capacity * sizeof(WCHAR*));
+        vals = HeapAlloc(GetProcessHeap(), 0, capacity * sizeof(LARGE_INTEGER));
+        if (!keys || !vals) {
+            WINE_ERR("Unable to allocate memory for labels cache!\n");
+            return FALSE;
+        }
+
+        SetFilePointer (context -> h, 0, NULL, FILE_BEGIN);
+        while (WCMD_fgets(string, sizeof(string)/sizeof(WCHAR), context->h)) {
+            str = string;
+            while (isspaceW (*str)) str++;
+            if (*str == ':') {
+                DWORD index = 0;
+                str++;
+                while (str[index] && (!isspaceW(str[index])))
+                    index++;
+                str[index] = '\0';
+
+                if (elems_count > 0 && (elems_count == capacity)) {
+                    capacity *= 2;
+                    keys = HeapReAlloc(GetProcessHeap(), 0, keys,
+                                       capacity * sizeof(WCHAR*));
+                    vals = HeapReAlloc(GetProcessHeap(), 0, vals,
+                                       capacity * sizeof(LARGE_INTEGER));
+                    if (!keys || !vals) {
+                        WINE_ERR("Unable to allocate memory for labels cache!\n");
+                        return FALSE;
+                    }
+                }
+
+                key = WCMD_strdupW(str);
+                keys[elems_count] = key;
+                val.QuadPart = 0;
+                val.u.LowPart = SetFilePointer(context -> h, val.u.LowPart,
+                                              &val.u.HighPart, FILE_CURRENT);
+                vals[elems_count] = val;
+                elems_count++;
+            }
+        }
+        cache_context->labels.names = keys;
+        cache_context->labels.positions = vals;
+        cache_context->labels.count = elems_count;
+    }
+
+    keys = cache_context->labels.names;
+    vals = cache_context->labels.positions;
+    elems_count = cache_context->labels.count;
+
+    /* A simple linear search should be quick and simple enough.
+     * Note: if the same label is "defined" twice, only the first one is used,
+     *       as Windows does.
+     */
+    for (i = 0; i < elems_count; i++) {
+        if (!lstrcmpiW(keys[i], label)) {
+            *pos = vals[i];
+            return TRUE;
+        }
+    }
+
+    return FALSE;
+}
+
 /****************************************************************************
- * WCMD_go_to
+ * WCMD_goto
  *
- * Batch file jump instruction. Not the most efficient algorithm ;-)
+ * Batch file jump instruction.
  * Prints error message if the specified label cannot be found - the file pointer is
  * then at EOF, effectively stopping the batch file.
  * FIXME: DOS is supposed to allow labels with spaces - we don't.
@@ -1353,9 +1456,6 @@ void WCMD_give_help (const WCHAR *command) {
 
 void WCMD_goto (CMD_LIST **cmdList) {
 
-  WCHAR string[MAX_PATH];
-  WCHAR current[MAX_PATH];
-
   /* Do not process any more parts of a processed multipart or multilines command */
   if (cmdList) *cmdList = NULL;
 
@@ -1364,8 +1464,10 @@ void WCMD_goto (CMD_LIST **cmdList) {
     return;
   }
   if (context != NULL) {
-    WCHAR *paramStart = param1, *str;
+    WCHAR *paramStart = param1;
     static const WCHAR eofW[] = {':','e','o','f','\0'};
+    BOOL label_found;
+    LARGE_INTEGER pos;
 
     /* Handle special :EOF label */
     if (lstrcmpiW (eofW, param1) == 0) {
@@ -1376,24 +1478,13 @@ void WCMD_goto (CMD_LIST **cmdList) {
     /* Support goto :label as well as goto label */
     if (*paramStart == ':') paramStart++;
 
-    SetFilePointer (context -> h, 0, NULL, FILE_BEGIN);
-    while (WCMD_fgets (string, sizeof(string)/sizeof(WCHAR), context -> h)) {
-      str = string;
-      while (isspaceW (*str)) str++;
-      if (*str == ':') {
-        DWORD index = 0;
-        str++;
-        while (((current[index] = str[index])) && (!isspaceW (current[index])))
-            index++;
-
-        /* ignore space at the end */
-        current[index] = 0;
-        if (lstrcmpiW (current, paramStart) == 0) return;
-      }
+    label_found = get_label_position(paramStart, &pos);
+    if (label_found) {
+        SetFilePointer(context->h, pos.u.LowPart, &pos.u.HighPart, FILE_BEGIN);
+    } else {
+        WCMD_output (WCMD_LoadMessage(WCMD_NOTARGET));
     }
-    WCMD_output (WCMD_LoadMessage(WCMD_NOTARGET));
   }
-  return;
 }
 
 /*****************************************************************************
diff --git a/programs/cmd/wcmd.h b/programs/cmd/wcmd.h
index bd07f5e..fede28c 100644
--- a/programs/cmd/wcmd.h
+++ b/programs/cmd/wcmd.h
@@ -118,6 +118,14 @@ void      WCMD_execute (const WCHAR *orig_command, const WCHAR *redirects,
                         const WCHAR *parameter, const WCHAR *substitution,
                         CMD_LIST **cmdList);
 
+/* Data structure to hold label/file position cache */
+
+typedef struct {
+    WCHAR **names;            /* List of labels, in file order */
+    LARGE_INTEGER *positions; /* Associated file offsets */
+    SIZE_T count;             /* Number of (labels/pos) stored */
+} CMD_LABELS;
+
 /* Data structure to hold context when executing batch files */
 
 typedef struct _BATCH_CONTEXT {
@@ -125,6 +133,8 @@ typedef struct _BATCH_CONTEXT {
   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 */
+  CMD_LABELS labels;    /* Labels and associated file positions (offsets) from
+                           beginning of the file */
   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 */
-- 
1.7.6



More information about the wine-tests-results mailing list