78603: Subject: [PATCH 2/2] msi: speed up WHERE statement evaluation by evaluating the condition as early as possible

buildbot at kegel.com buildbot at kegel.com
Fri Sep 9 04:40:23 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-gcc295 while building Wine.
Full details are available at: http://buildbot.kegel.com/builders/runtests-gcc295/builds/14 (though maybe not for long, as I'm still reinstalling the buildbot periodically while experimenting)
BUILD FAILED: failed shell_2


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


-------------- next part --------------
From: Bernhard Loos <bernhardloos at googlemail.com>
Subject: [PATCH 1/2] msi/where.c: the underlying tables might have changed, so it's not possible to cache the result of the execute
Message-Id: <CAOB12PVJSaikL6HWY1tJczNpbUx3BW+NDDX8bKHw5TKQVcEdWQ at mail.gmail.com>
Date: Fri, 9 Sep 2011 11:25:51 +0200

---
 dlls/msi/tests/db.c |   24 +++++++++++++++++++++++-
 dlls/msi/where.c    |   35 ++++++++++++++---------------------
 2 files changed, 37 insertions(+), 22 deletions(-)

diff --git a/dlls/msi/tests/db.c b/dlls/msi/tests/db.c
index b581633..b915fa0 100644
--- a/dlls/msi/tests/db.c
+++ b/dlls/msi/tests/db.c
@@ -297,7 +297,7 @@ make_add_entry(binary,
 
 static void test_msiinsert(void)
 {
-    MSIHANDLE hdb = 0, hview = 0, hrec = 0;
+    MSIHANDLE hdb = 0, hview = 0, hview2 = 0, hrec = 0;
     UINT r;
     const char *query;
     char buf[80];
@@ -322,6 +322,14 @@ static void test_msiinsert(void)
     r = MsiCloseHandle(hview);
     ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n");
 
+    query = "SELECT * FROM phone WHERE number = '8675309'";
+    r = MsiDatabaseOpenView(hdb, query, &hview2);
+    ok(r == ERROR_SUCCESS, "MsiDatabaseOpenView failed\n");
+    r = MsiViewExecute(hview2, 0);
+    ok(r == ERROR_SUCCESS, "MsiViewExecute failed\n");
+    r = MsiViewFetch(hview2, &hrec);
+    ok(r == ERROR_NO_MORE_ITEMS, "MsiViewFetch produced items\n");
+
     /* insert a value into it */
     query = "INSERT INTO `phone` ( `id`, `name`, `number` )"
         "VALUES('1', 'Abe', '8675309')";
@@ -334,6 +342,20 @@ static void test_msiinsert(void)
     r = MsiCloseHandle(hview);
     ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n");
 
+    r = MsiViewFetch(hview2, &hrec);
+    ok(r == ERROR_NO_MORE_ITEMS, "MsiViewFetch produced items\n");
+    r = MsiViewExecute(hview2, 0);
+    ok(r == ERROR_SUCCESS, "MsiViewExecute failed\n");
+    r = MsiViewFetch(hview2, &hrec);
+    ok(r == ERROR_SUCCESS, "MsiViewFetch failed: %u\n", r);
+
+    r = MsiCloseHandle(hrec);
+    ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n");
+    r = MsiViewClose(hview2);
+    ok(r == ERROR_SUCCESS, "MsiViewClose failed\n");
+    r = MsiCloseHandle(hview2);
+    ok(r == ERROR_SUCCESS, "MsiCloseHandle failed\n");
+
     query = "SELECT * FROM `phone` WHERE `id` = 1";
     r = do_query(hdb, query, &hrec);
     ok(r == ERROR_SUCCESS, "MsiViewFetch failed\n");
diff --git a/dlls/msi/where.c b/dlls/msi/where.c
index 0e346d6..17b655b 100644
--- a/dlls/msi/where.c
+++ b/dlls/msi/where.c
@@ -73,27 +73,10 @@ typedef struct tagMSIWHEREVIEW
     struct expr   *cond;
     UINT           rec_index;
     MSIORDERINFO  *order_info;
-    UINT           error;
 } MSIWHEREVIEW;
 
 #define INITIAL_REORDER_SIZE 16
 
-static UINT init_reorder(MSIWHEREVIEW *wv)
-{
-    MSIROWENTRY **new = msi_alloc_zero(sizeof(MSIROWENTRY *) * INITIAL_REORDER_SIZE);
-    if (!new)
-        return ERROR_OUTOFMEMORY;
-
-    if (wv->reorder)
-        msi_free(wv->reorder);
-
-    wv->reorder = new;
-    wv->reorder_size = INITIAL_REORDER_SIZE;
-    wv->row_count = 0;
-
-    return ERROR_SUCCESS;
-}
-
 static void free_reorder(MSIWHEREVIEW *wv)
 {
     UINT i;
@@ -110,6 +93,20 @@ static void free_reorder(MSIWHEREVIEW *wv)
     wv->row_count = 0;
 }
 
+static UINT init_reorder(MSIWHEREVIEW *wv)
+{
+    MSIROWENTRY **new = msi_alloc_zero(sizeof(MSIROWENTRY *) * INITIAL_REORDER_SIZE);
+    if (!new)
+        return ERROR_OUTOFMEMORY;
+
+    free_reorder(wv);
+
+    wv->reorder = new;
+    wv->reorder_size = INITIAL_REORDER_SIZE;
+
+    return ERROR_SUCCESS;
+}
+
 static inline UINT find_row(MSIWHEREVIEW *wv, UINT row, UINT *(values[]))
 {
     if (row >= wv->row_count)
@@ -618,9 +615,6 @@ static UINT WHERE_execute( struct tagMSIVIEW *view, MSIRECORD *record )
     if( !table )
          return ERROR_FUNCTION_FAILED;
 
-    if (wv->reorder)
-        return wv->error;
-
     r = init_reorder(wv);
     if (r != ERROR_SUCCESS)
         return r;
@@ -654,7 +648,6 @@ static UINT WHERE_execute( struct tagMSIVIEW *view, MSIRECORD *record )
         r = wv->order_info->error;
 
     msi_free( rows );
-    wv->error = r;
     return r;
 }
 

From: Bernhard Loos <bernhardloos at googlemail.com>
Subject: [PATCH 2/2] msi: speed up WHERE statement evaluation by evaluating the condition as early as possible
Message-Id: <CAOB12PWMqFFZW_u+tm=q5yZPAE5fs3pnw-GjooLW__iFHDiL0A at mail.gmail.com>
Date: Fri, 9 Sep 2011 11:25:58 +0200

---
 dlls/msi/where.c |  208 +++++++++++++++++++++++++++++++++++++----------------
 1 files changed, 145 insertions(+), 63 deletions(-)

diff --git a/dlls/msi/where.c b/dlls/msi/where.c
index 17b655b..875bebb 100644
--- a/dlls/msi/where.c
+++ b/dlls/msi/where.c
@@ -2,6 +2,7 @@
  * Implementation of the Microsoft Installer (msi.dll)
  *
  * Copyright 2002 Mike McCormack for CodeWeavers
+ * Copyright 2011 Bernhard Loos
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -75,8 +76,13 @@ typedef struct tagMSIWHEREVIEW
     MSIORDERINFO  *order_info;
 } MSIWHEREVIEW;
 
+static UINT WHERE_evaluate( MSIWHEREVIEW *wv, const UINT rows[],
+                            struct expr *cond, INT *val, MSIRECORD *record );
+
 #define INITIAL_REORDER_SIZE 16
 
+#define INVALID_ROW_INDEX -1
+
 static void free_reorder(MSIWHEREVIEW *wv)
 {
     UINT i;
@@ -362,90 +368,170 @@ static UINT WHERE_delete_row(struct tagMSIVIEW *view, UINT row)
     return wv->tables->view->ops->delete_row(wv->tables->view, rows[0]);
 }
 
-static INT INT_evaluate_binary( INT lval, UINT op, INT rval )
+static INT INT_evaluate_binary( MSIWHEREVIEW *wv, const UINT rows[],
+                                const struct complex_expr *expr, INT *val, MSIRECORD *record )
 {
-    switch( op )
+    UINT rl, rr;
+    INT lval, rval;
+
+    rl = WHERE_evaluate(wv, rows, expr->left, &lval, record);
+    if (rl != ERROR_SUCCESS && rl != ERROR_CONTINUE)
+        return rl;
+    rr = WHERE_evaluate(wv, rows, expr->right, &rval, record);
+    if (rr != ERROR_SUCCESS && rr != ERROR_CONTINUE)
+        return rr;
+
+    if (rl == ERROR_CONTINUE || rr == ERROR_CONTINUE)
+    {
+        if (rl == rr)
+        {
+            *val = TRUE;
+            return ERROR_CONTINUE;
+        }
+
+        if (expr->op == OP_AND)
+        {
+            if ((rl == ERROR_CONTINUE && !rval) || (rr == ERROR_CONTINUE && !lval))
+            {
+                *val = FALSE;
+                return ERROR_SUCCESS;
+            }
+        }
+        else if (expr->op == OP_OR)
+        {
+            if ((rl == ERROR_CONTINUE && rval) || (rr == ERROR_CONTINUE && lval))
+            {
+                *val = TRUE;
+                return ERROR_SUCCESS;
+            }
+        }
+
+        *val = TRUE;
+        return ERROR_CONTINUE;
+    }
+
+    switch( expr->op )
     {
     case OP_EQ:
-        return ( lval == rval );
+        *val = ( lval == rval );
+        break;
     case OP_AND:
-        return ( lval && rval );
+        *val = ( lval && rval );
+        break;
     case OP_OR:
-        return ( lval || rval );
+        *val = ( lval || rval );
+        break;
     case OP_GT:
-        return ( lval > rval );
+        *val = ( lval > rval );
+        break;
     case OP_LT:
-        return ( lval < rval );
+        *val = ( lval < rval );
+        break;
     case OP_LE:
-        return ( lval <= rval );
+        *val = ( lval <= rval );
+        break;
     case OP_GE:
-        return ( lval >= rval );
+        *val = ( lval >= rval );
+        break;
     case OP_NE:
-        return ( lval != rval );
+        *val = ( lval != rval );
+        break;
     default:
-        ERR("Unknown operator %d\n", op );
+        ERR("Unknown operator %d\n", expr->op );
+        return ERROR_FUNCTION_FAILED;
     }
-    return 0;
+
+    return ERROR_SUCCESS;
 }
 
 static inline UINT expr_fetch_value(const union ext_column *expr, const UINT rows[], UINT *val)
 {
     JOINTABLE *table = expr->parsed.table;
 
+    if( rows[table->table_index] == INVALID_ROW_INDEX )
+    {
+        *val = 1;
+        return ERROR_CONTINUE;
+    }
     return table->view->ops->fetch_int(table->view, rows[table->table_index],
                                         expr->parsed.column, val);
 }
 
 
-static INT INT_evaluate_unary( INT lval, UINT op )
+static UINT INT_evaluate_unary( MSIWHEREVIEW *wv, const UINT rows[],
+                                const struct complex_expr *expr, INT *val, MSIRECORD *record )
 {
-    switch( op )
+    UINT r;
+    UINT lval;
+
+    r = expr_fetch_value(&expr->left->u.column, rows, &lval);
+    if(r != ERROR_SUCCESS)
+        return r;
+
+    switch( expr->op )
     {
     case OP_ISNULL:
-        return ( !lval );
+        *val = !lval;
+        break;
     case OP_NOTNULL:
-        return ( lval );
+        *val = lval;
+        break;
     default:
-        ERR("Unknown operator %d\n", op );
+        ERR("Unknown operator %d\n", expr->op );
+        return ERROR_FUNCTION_FAILED;
     }
-    return 0;
+    return ERROR_SUCCESS;
 }
 
-static const WCHAR *STRING_evaluate( MSIWHEREVIEW *wv, const UINT rows[],
+static UINT STRING_evaluate( MSIWHEREVIEW *wv, const UINT rows[],
                                      const struct expr *expr,
-                                     const MSIRECORD *record )
+                                     const MSIRECORD *record,
+                                     const WCHAR **str )
 {
-    UINT val = 0, r;
+    UINT val = 0, r = ERROR_SUCCESS;
 
     switch( expr->type )
     {
     case EXPR_COL_NUMBER_STRING:
         r = expr_fetch_value(&expr->u.column, rows, &val);
-        if( r != ERROR_SUCCESS )
-            return NULL;
-        return msi_string_lookup_id( wv->db->strings, val );
+        if (r == ERROR_SUCCESS)
+            *str =  msi_string_lookup_id(wv->db->strings, val);
+        else
+            *str = NULL;
+        break;
 
     case EXPR_SVAL:
-        return expr->u.sval;
+        *str = expr->u.sval;
+        break;
 
     case EXPR_WILDCARD:
-        return MSI_RecordGetString( record, ++wv->rec_index );
+        *str = MSI_RecordGetString(record, ++wv->rec_index);
+        break;
 
     default:
         ERR("Invalid expression type\n");
+        r = ERROR_FUNCTION_FAILED;
+        *str = NULL;
         break;
     }
-    return NULL;
+    return r;
 }
 
-static UINT STRCMP_Evaluate( MSIWHEREVIEW *wv, const UINT rows[], const struct expr *cond,
+static UINT STRCMP_Evaluate( MSIWHEREVIEW *wv, const UINT rows[], const struct complex_expr *expr,
                              INT *val, const MSIRECORD *record )
 {
     int sr;
     const WCHAR *l_str, *r_str;
+    UINT r;
+
+    *val = TRUE;
+    r = STRING_evaluate(wv, rows, expr->left, record, &l_str);
+    if (r == ERROR_CONTINUE)
+        return r;
+    r = STRING_evaluate(wv, rows, expr->right, record, &r_str);
+    if (r == ERROR_CONTINUE)
+        return r;
 
-    l_str = STRING_evaluate( wv, rows, cond->u.expr.left, record );
-    r_str = STRING_evaluate( wv, rows, cond->u.expr.right, record );
     if( l_str == r_str ||
         ((!l_str || !*l_str) && (!r_str || !*r_str)) )
         sr = 0;
@@ -456,8 +542,8 @@ static UINT STRCMP_Evaluate( MSIWHEREVIEW *wv, const UINT rows[], const struct e
     else
         sr = strcmpW( l_str, r_str );
 
-    *val = ( cond->u.expr.op == OP_EQ && ( sr == 0 ) ) ||
-           ( cond->u.expr.op == OP_NE && ( sr != 0 ) );
+    *val = ( expr->op == OP_EQ && ( sr == 0 ) ) ||
+           ( expr->op == OP_NE && ( sr != 0 ) );
 
     return ERROR_SUCCESS;
 }
@@ -466,7 +552,6 @@ static UINT WHERE_evaluate( MSIWHEREVIEW *wv, const UINT rows[],
                             struct expr *cond, INT *val, MSIRECORD *record )
 {
     UINT r, tval;
-    INT lval, rval;
 
     if( !cond )
     {
@@ -495,24 +580,13 @@ static UINT WHERE_evaluate( MSIWHEREVIEW *wv, const UINT rows[],
         return ERROR_SUCCESS;
 
     case EXPR_COMPLEX:
-        r = WHERE_evaluate( wv, rows, cond->u.expr.left, &lval, record );
-        if( r != ERROR_SUCCESS )
-            return r;
-        r = WHERE_evaluate( wv, rows, cond->u.expr.right, &rval, record );
-        if( r != ERROR_SUCCESS )
-            return r;
-        *val = INT_evaluate_binary( lval, cond->u.expr.op, rval );
-        return ERROR_SUCCESS;
+        return INT_evaluate_binary(wv, rows, &cond->u.expr, val, record);
 
     case EXPR_UNARY:
-        r = expr_fetch_value(&cond->u.expr.left->u.column, rows, &tval);
-        if( r != ERROR_SUCCESS )
-            return r;
-        *val = INT_evaluate_unary( tval, cond->u.expr.op );
-        return ERROR_SUCCESS;
+        return INT_evaluate_unary( wv, rows, &cond->u.expr, val, record );
 
     case EXPR_STRCMP:
-        return STRCMP_Evaluate( wv, rows, cond, val, record );
+        return STRCMP_Evaluate( wv, rows, &cond->u.expr, val, record );
 
     case EXPR_WILDCARD:
         *val = MSI_RecordGetInteger( record, ++wv->rec_index );
@@ -533,26 +607,30 @@ static UINT check_condition( MSIWHEREVIEW *wv, MSIRECORD *record, JOINTABLE *tab
     INT val;
 
     for (table_rows[table->table_index] = 0; table_rows[table->table_index] < table->row_count;
-          table_rows[table->table_index]++)
+         table_rows[table->table_index]++)
     {
-        if (table->next)
-        {
-            r = check_condition(wv, record, table->next, table_rows);
-            if(r != ERROR_SUCCESS)
-                break;
-        }
-        else
+        val = 0;
+        wv->rec_index = 0;
+        r = WHERE_evaluate( wv, table_rows, wv->cond, &val, record );
+        if (r != ERROR_SUCCESS && r != ERROR_CONTINUE)
+            break;
+        if (val)
         {
-            val = 0;
-            wv->rec_index = 0;
-            r = WHERE_evaluate (wv, table_rows, wv->cond, &val, record);
-            if(r != ERROR_SUCCESS)
-                break;
-            if (!val)
-                continue;
-            add_row(wv, table_rows);
+            if (table->next)
+            {
+                r = check_condition(wv, record, table->next, table_rows);
+                if (r != ERROR_SUCCESS)
+                    break;
+            }
+            else
+            {
+                if (r != ERROR_SUCCESS)
+                    break;
+                add_row (wv, table_rows);
+            }
         }
     }
+    table_rows[table->table_index] = INVALID_ROW_INDEX;
     return r;
 }
 
@@ -609,6 +687,7 @@ static UINT WHERE_execute( struct tagMSIVIEW *view, MSIRECORD *record )
     UINT r;
     JOINTABLE *table = wv->tables;
     UINT *rows;
+    int i;
 
     TRACE("%p %p\n", wv, record);
 
@@ -637,6 +716,9 @@ static UINT WHERE_execute( struct tagMSIVIEW *view, MSIRECORD *record )
     while ((table = table->next));
 
     rows = msi_alloc( wv->table_count * sizeof(*rows) );
+    for (i = 0; i < wv->table_count; i++)
+        rows[i] = INVALID_ROW_INDEX;
+
     r =  check_condition(wv, record, wv->tables, rows);
 
     if (wv->order_info)



More information about the wine-tests-results mailing list