Malcolm Rowe wrote:
> On Thu, Aug 31, 2006 at 12:46:07AM +0530, Kamesh Jayachandran wrote:
>
>> Daniel Berlin wrote:
>>
>>> This is fine (though mildly pointless because without being able to
>>> move the entire prepared statement out of the loop, it's probably not
>>> much of a speedup if at all)
>>>
>>>
>> I don't think so. we save the compilation cost. I believe the prepared
>> statements are meant to be compiled once and the place holder values to
>> be substituted based on the need(most often from the loop).
>>
>> Correct me if I am wrong.
>>
>>
>
> That's the way that prepare/execute works in general.
>
> Whether it actually has any effect in this case, you'd need to confirm
> by testing (and as I think that Dan is saying, it's unlikely to make
> any really significant difference in this case, at which point you
> probably want to start balancing code readability against marginal
> efficiency gains).
>
>
Snip from http://www.linuxjournal.com/article/7803
<snip>
The question mark represents a wildcard, a place where real data is
bound before the query actually runs. The query is prepared once, and
for each execution, actual data is bound to the wildcards. This
eliminates the need for careful, paranoid parsing of user-supplied data
for killer characters and potential exploits.
</snip>
I already have 20 merges on /trunk from 20 different branches.
When I did my 21'st merge+commit(21 insertions), I observed the following
with patch
------------------
real 0m0.506s
user 0m0.049s
sys 0m0.022s
without patch
------------------
real 0m1.181s
user 0m0.038s
sys 0m0.027s
I fail to see the point it breaks the code readability, the user at this
code should see only 'mergedrevstart' and 'mergedrevend' are variants
across iterations rest are constants across iterations.
I could not understand how putting (statement creation, all binds, step,
finalize together in the loop) will improve the readability!
> Also, can sqlite3_reset never return an error? You're not checking
> for one.
>
>
Thanks.
Attached patch takes care of that.
With regards
Kamesh Jayachandran
[[[
Patch by: Kamesh Jayachandran <kamesh@collab.net>
Use the same prepared statement across multiple queries of same kind.
Bind the invariant values outside the 'for loop'.
* subversion/libsvn_fs_fs/fs_fs.c
(index_path_merge_info):
Create the statement outside the for loop.
Reset the statement after each execution as mandated by sqlite
using sqlite3_reset(Note: reset preserves the bound value, it just
resets the state of the statement for reexecution.)
Bind 'mergedfrom', 'mergedto', 'revision' outside the for loop as they
are invariants across iterations.
Finalize the statement outside the 'for loop'.
]]]
Index: subversion/libsvn_fs_fs/fs_fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.c (revision 21302)
+++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
@@ -4022,20 +4022,19 @@
if (from && revlist)
{
int i;
+ SQLITE_ERR(sqlite3_prepare(ftd->mtd,
+ "INSERT INTO mergeinfo (revision, mergedto, mergedfrom, mergedrevstart, mergedrevend) VALUES (?, ?, ?, ?, ?);",
+ -1, &stmt, NULL), ftd->mtd);
+ SQLITE_ERR(sqlite3_bind_int64(stmt, 1, new_rev), ftd->mtd);
+ SQLITE_ERR(sqlite3_bind_text(stmt, 2, path, -1, SQLITE_TRANSIENT),
+ ftd->mtd);
+ SQLITE_ERR(sqlite3_bind_text(stmt, 3, from, -1, SQLITE_TRANSIENT),
+ ftd->mtd);
for (i = 0; i < revlist->nelts; i++)
{
svn_merge_range_t *range;
range = APR_ARRAY_IDX(revlist, i, svn_merge_range_t *);
- SQLITE_ERR(sqlite3_prepare(ftd->mtd,
- "INSERT INTO mergeinfo (revision, mergedto, mergedfrom, mergedrevstart, mergedrevend) VALUES (?, ?, ?, ?, ?);",
- -1, &stmt, NULL), ftd->mtd);
- SQLITE_ERR(sqlite3_bind_int64(stmt, 1, new_rev),
- ftd->mtd);
- SQLITE_ERR(sqlite3_bind_text(stmt, 2, path, -1, SQLITE_TRANSIENT),
- ftd->mtd);
- SQLITE_ERR(sqlite3_bind_text(stmt, 3, from, -1, SQLITE_TRANSIENT),
- ftd->mtd);
SQLITE_ERR(sqlite3_bind_int64(stmt, 4, range->start),
ftd->mtd);
SQLITE_ERR(sqlite3_bind_int64(stmt, 5, range->end),
@@ -4043,9 +4042,10 @@
if (sqlite3_step(stmt) != SQLITE_DONE)
return svn_error_create(SVN_ERR_FS_SQLITE_ERROR, NULL,
sqlite3_errmsg(ftd->mtd));
-
- SQLITE_ERR(sqlite3_finalize(stmt), ftd->mtd);
+
+ SQLITE_ERR(sqlite3_reset(stmt), ftd->mtd);
}
+ SQLITE_ERR(sqlite3_finalize(stmt), ftd->mtd);
}
}
SQLITE_ERR (sqlite3_prepare(ftd->mtd,
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Aug 31 11:28:23 2006