You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2012/05/23 03:31:10 UTC

svn commit: r1341715 - in /subversion/trunk/subversion: libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c tests/libsvn_wc/wc-queries-test.c

Author: rhuijben
Date: Wed May 23 01:31:09 2012
New Revision: 1341715

URL: http://svn.apache.org/viewvc?rev=1341715&view=rev
Log:
Split the root node in the target list processing from its children and/or
descendants to avoid a table scan.

Two cheap queries are together much faster than a table scan.

* subversion/libsvn_wc/wc-queries.sql
  (STMT_INSERT_TARGET): Reformat.
  (STMT_INSERT_TARGET_DEPTH_FILES): Only select files from children.
  (STMT_INSERT_TARGET_DEPTH_IMMEDIATES): Only select children.
  (STMT_INSERT_TARGET_DEPTH_INFINITY): Only select descendants
    (to work the other 7).

  (STMT_INSERT_TARGET_WITH_CHANGELIST): Reformat.
  (STMT_INSERT_TARGET_WITH_CHANGELIST_DEPTH_FILES):
    Only select files from children.
  (STMT_INSERT_TARGET_WITH_CHANGELIST_DEPTH_IMMEDIATES): Only select children.
  (STMT_INSERT_TARGET_WITH_CHANGELIST_DEPTH_INFINITY): Only select descendants.

* subversion/libsvn_wc/wc_db.c
  (populate_targets_tree): Separate the root from the rest query to improve
    performance on large working copies by using the index.

* subversion/tests/libsvn_wc/wc-queries-test.c
  (slow_statements): Remove 6 more previously slow queries.

Modified:
    subversion/trunk/subversion/libsvn_wc/wc-queries.sql
    subversion/trunk/subversion/libsvn_wc/wc_db.c
    subversion/trunk/subversion/tests/libsvn_wc/wc-queries-test.c

Modified: subversion/trunk/subversion/libsvn_wc/wc-queries.sql
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc-queries.sql?rev=1341715&r1=1341714&r2=1341715&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc-queries.sql (original)
+++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Wed May 23 01:31:09 2012
@@ -479,51 +479,59 @@ DROP TABLE IF EXISTS targets_list
 -- STMT_INSERT_TARGET
 INSERT INTO targets_list(wc_id, local_relpath, parent_relpath, kind)
 SELECT wc_id, local_relpath, parent_relpath, kind
-FROM nodes_current WHERE wc_id = ?1 AND local_relpath = ?2
+FROM nodes_current
+WHERE wc_id = ?1
+  AND local_relpath = ?2
 
 -- STMT_INSERT_TARGET_DEPTH_FILES
 INSERT INTO targets_list(wc_id, local_relpath, parent_relpath, kind)
 SELECT wc_id, local_relpath, parent_relpath, kind
 FROM nodes_current
-WHERE wc_id = ?1 AND ((parent_relpath = ?2 AND kind = 'file')
-                      OR local_relpath = ?2)
+WHERE wc_id = ?1
+  AND parent_relpath = ?2
+  AND kind = 'file'
 
 -- STMT_INSERT_TARGET_DEPTH_IMMEDIATES
 INSERT INTO targets_list(wc_id, local_relpath, parent_relpath, kind)
 SELECT wc_id, local_relpath, parent_relpath, kind
 FROM nodes_current
-WHERE wc_id = ?1 AND (parent_relpath = ?2 OR local_relpath = ?2)
+WHERE wc_id = ?1
+  AND parent_relpath = ?2
 
 -- STMT_INSERT_TARGET_DEPTH_INFINITY
 INSERT INTO targets_list(wc_id, local_relpath, parent_relpath, kind)
 SELECT wc_id, local_relpath, parent_relpath, kind
 FROM nodes_current
 WHERE wc_id = ?1
-       AND (local_relpath = ?2 
-            OR IS_STRICT_DESCENDANT_OF(local_relpath, ?2))
+  AND IS_STRICT_DESCENDANT_OF(local_relpath, ?2)
 
 -- STMT_INSERT_TARGET_WITH_CHANGELIST
 INSERT INTO targets_list(wc_id, local_relpath, parent_relpath, kind)
 SELECT N.wc_id, N.local_relpath, N.parent_relpath, N.kind
   FROM actual_node AS A JOIN nodes_current AS N
     ON A.wc_id = N.wc_id AND A.local_relpath = N.local_relpath
- WHERE N.wc_id = ?1 AND A.changelist = ?3 AND N.local_relpath = ?2
+ WHERE N.wc_id = ?1
+   AND N.local_relpath = ?2
+   AND A.changelist = ?3
 
 -- STMT_INSERT_TARGET_WITH_CHANGELIST_DEPTH_FILES
 INSERT INTO targets_list(wc_id, local_relpath, parent_relpath, kind)
 SELECT N.wc_id, N.local_relpath, N.parent_relpath, N.kind
   FROM actual_node AS A JOIN nodes_current AS N
     ON A.wc_id = N.wc_id AND A.local_relpath = N.local_relpath
- WHERE N.wc_id = ?1 AND A.changelist = ?3
-       AND ((N.parent_relpath = ?2 AND kind = 'file') OR N.local_relpath = ?2)
+ WHERE N.wc_id = ?1
+   AND N.parent_relpath = ?2
+   AND kind = 'file'
+   AND A.changelist = ?3
 
 -- STMT_INSERT_TARGET_WITH_CHANGELIST_DEPTH_IMMEDIATES
 INSERT INTO targets_list(wc_id, local_relpath, parent_relpath, kind)
 SELECT N.wc_id, N.local_relpath, N.parent_relpath, N.kind
   FROM actual_node AS A JOIN nodes_current AS N
     ON A.wc_id = N.wc_id AND A.local_relpath = N.local_relpath
- WHERE N.wc_id = ?1 AND A.changelist = ?3
-       AND (N.parent_relpath = ?2 OR N.local_relpath = ?2)
+ WHERE N.wc_id = ?1
+   AND N.parent_relpath = ?2
+  AND A.changelist = ?3
 
 -- STMT_INSERT_TARGET_WITH_CHANGELIST_DEPTH_INFINITY
 INSERT INTO targets_list(wc_id, local_relpath, parent_relpath, kind)
@@ -531,9 +539,8 @@ SELECT N.wc_id, N.local_relpath, N.paren
   FROM actual_node AS A JOIN nodes_current AS N
     ON A.wc_id = N.wc_id AND A.local_relpath = N.local_relpath
  WHERE N.wc_id = ?1
-       AND (N.local_relpath = ?2 
-            OR IS_STRICT_DESCENDANT_OF(N.local_relpath, ?2))
-       AND A.changelist = ?3
+   AND IS_STRICT_DESCENDANT_OF(N.local_relpath, ?2)
+   AND A.changelist = ?3
 
 -- STMT_SELECT_TARGETS
 SELECT local_relpath, parent_relpath from targets_list

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1341715&r1=1341714&r2=1341715&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Wed May 23 01:31:09 2012
@@ -4947,17 +4947,29 @@ populate_targets_tree(svn_wc__db_wcroot_
           const char *changelist = APR_ARRAY_IDX(changelist_filter, i,
                                                  const char *);
 
-          SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb, stmt_idx));
+          SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
+                                        STMT_INSERT_TARGET_WITH_CHANGELIST));
           SVN_ERR(svn_sqlite__bindf(stmt, "iss", wcroot->wc_id,
                                     local_relpath, changelist));
           SVN_ERR(svn_sqlite__update(&sub_affected, stmt));
 
+          /* If the root is matched by the changelist, we don't have to match
+             the children. As that tells us the root is a file */
+          if (!sub_affected && depth > svn_depth_empty)
+            {
+              SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb, stmt_idx));
+              SVN_ERR(svn_sqlite__bindf(stmt, "iss", wcroot->wc_id,
+                                        local_relpath, changelist));
+              SVN_ERR(svn_sqlite__update(&sub_affected, stmt));
+            }
+
           affected_rows += sub_affected;
         }
     }
   else /* No changelist filtering */
     {
       int stmt_idx;
+      int sub_affected;
 
       switch (depth)
         {
@@ -4983,9 +4995,19 @@ populate_targets_tree(svn_wc__db_wcroot_
             break;
         }
 
-      SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb, stmt_idx));
+      SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
+                                        STMT_INSERT_TARGET));
       SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
-      SVN_ERR(svn_sqlite__update(&affected_rows, stmt));
+      SVN_ERR(svn_sqlite__update(&sub_affected, stmt));
+      affected_rows += sub_affected;
+
+      if (depth > svn_depth_empty)
+        {
+          SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb, stmt_idx));
+          SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
+          SVN_ERR(svn_sqlite__update(&sub_affected, stmt));
+          affected_rows += sub_affected;
+        }
     }
 
   /* Does the target exist? */

Modified: subversion/trunk/subversion/tests/libsvn_wc/wc-queries-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_wc/wc-queries-test.c?rev=1341715&r1=1341714&r2=1341715&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_wc/wc-queries-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_wc/wc-queries-test.c Wed May 23 01:31:09 2012
@@ -92,12 +92,6 @@ static const int slow_statements[] =
   STMT_SELECT_CHANGELIST_LIST,
 
   /* Need review: */
-  STMT_INSERT_TARGET_DEPTH_FILES,
-  STMT_INSERT_TARGET_DEPTH_IMMEDIATES,
-  STMT_INSERT_TARGET_DEPTH_INFINITY,
-  STMT_INSERT_TARGET_WITH_CHANGELIST_DEPTH_FILES,
-  STMT_INSERT_TARGET_WITH_CHANGELIST_DEPTH_IMMEDIATES,
-  STMT_INSERT_TARGET_WITH_CHANGELIST_DEPTH_INFINITY,
   STMT_SELECT_COMMITTABLE_EXTERNALS_BELOW,
   STMT_SELECT_EXTERNALS_DEFINED,
   STMT_SELECT_EXTERNAL_PROPERTIES,



RE: svn commit: r1341715 - in /subversion/trunk/subversion: libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c tests/libsvn_wc/wc-queries-test.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Greg Stein [mailto:gstein@gmail.com]
> Sent: woensdag 23 mei 2012 4:54
> To: dev@subversion.apache.org
> Subject: Re: svn commit: r1341715 - in /subversion/trunk/subversion:
> libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c
tests/libsvn_wc/wc-queries-test.c
> 
> On Tue, May 22, 2012 at 9:31 PM,  <rh...@apache.org> wrote:
> >...
> > +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Wed May 23 01:31:09
> 2012
> > @@ -4947,17 +4947,29 @@ populate_targets_tree(svn_wc__db_wcroot_
> >           const char *changelist = APR_ARRAY_IDX(changelist_filter, i,
> >                                                  const char *);
> >
> > -          SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
stmt_idx));
> > +          SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
> > +                                      
 STMT_INSERT_TARGET_WITH_CHANGELIST));
> >           SVN_ERR(svn_sqlite__bindf(stmt, "iss", wcroot->wc_id,
> >                                     local_relpath, changelist));
> >           SVN_ERR(svn_sqlite__update(&sub_affected, stmt));
> >
> > +          /* If the root is matched by the changelist, we don't have to
match
> > +             the children. As that tells us the root is a file */
> > +          if (!sub_affected && depth > svn_depth_empty)
> > +            {
> > +              SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
stmt_idx));
> > +              SVN_ERR(svn_sqlite__bindf(stmt, "iss", wcroot->wc_id,
> > +                                        local_relpath, changelist));
> > +              SVN_ERR(svn_sqlite__update(&sub_affected, stmt));
> > +            }
> > +
> >           affected_rows += sub_affected;
> >         }
> >     }
> >   else /* No changelist filtering */
> >     {
> >       int stmt_idx;
> > +      int sub_affected;
> >
> >       switch (depth)
> >         {
> > @@ -4983,9 +4995,19 @@ populate_targets_tree(svn_wc__db_wcroot_
> >             break;
> >         }
> >
> > -      SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb, stmt_idx));
> > +      SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
> > +                                        STMT_INSERT_TARGET));
> >       SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id,
local_relpath));
> > -      SVN_ERR(svn_sqlite__update(&affected_rows, stmt));
> > +      SVN_ERR(svn_sqlite__update(&sub_affected, stmt));
> > +      affected_rows += sub_affected;
> > +
> > +      if (depth > svn_depth_empty)
> > +        {
> 
> Why is sub_affected not used here? (like above) ... should a comment
> be added to explain?

This just inserts the directory itself. The previous statement (which has a
comment) will only insert a node when it is not a directory. Directories
still can't be part of a changelist.

I just used the result 'did we insert a file' as a quick win to avoid
looking for changelists on children of files.

	Bert
> 
> >...
> 
> Cheers,
> -g


Re: svn commit: r1341715 - in /subversion/trunk/subversion: libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c tests/libsvn_wc/wc-queries-test.c

Posted by Greg Stein <gs...@gmail.com>.
On Tue, May 22, 2012 at 9:31 PM,  <rh...@apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Wed May 23 01:31:09 2012
> @@ -4947,17 +4947,29 @@ populate_targets_tree(svn_wc__db_wcroot_
>           const char *changelist = APR_ARRAY_IDX(changelist_filter, i,
>                                                  const char *);
>
> -          SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb, stmt_idx));
> +          SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
> +                                        STMT_INSERT_TARGET_WITH_CHANGELIST));
>           SVN_ERR(svn_sqlite__bindf(stmt, "iss", wcroot->wc_id,
>                                     local_relpath, changelist));
>           SVN_ERR(svn_sqlite__update(&sub_affected, stmt));
>
> +          /* If the root is matched by the changelist, we don't have to match
> +             the children. As that tells us the root is a file */
> +          if (!sub_affected && depth > svn_depth_empty)
> +            {
> +              SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb, stmt_idx));
> +              SVN_ERR(svn_sqlite__bindf(stmt, "iss", wcroot->wc_id,
> +                                        local_relpath, changelist));
> +              SVN_ERR(svn_sqlite__update(&sub_affected, stmt));
> +            }
> +
>           affected_rows += sub_affected;
>         }
>     }
>   else /* No changelist filtering */
>     {
>       int stmt_idx;
> +      int sub_affected;
>
>       switch (depth)
>         {
> @@ -4983,9 +4995,19 @@ populate_targets_tree(svn_wc__db_wcroot_
>             break;
>         }
>
> -      SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb, stmt_idx));
> +      SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
> +                                        STMT_INSERT_TARGET));
>       SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath));
> -      SVN_ERR(svn_sqlite__update(&affected_rows, stmt));
> +      SVN_ERR(svn_sqlite__update(&sub_affected, stmt));
> +      affected_rows += sub_affected;
> +
> +      if (depth > svn_depth_empty)
> +        {

Why is sub_affected not used here? (like above) ... should a comment
be added to explain?

>...

Cheers,
-g