On Thu, Aug 31, 2006 at 02:57:17PM +0530, Kamesh Jayachandran wrote:
> >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>
>
Yes, and? I do understand how prepare/execute works.
> 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
>
Thank you. _That_ is the information you should have included with
your first patch submission.
So, assuming that's a realistic use case, it appears that there's a
genuine performance reason to split apart the prepare and execute steps.
Dan, is that a fair comment?
> 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!
>
Keeping the code to build, bind and execute the query together means that
it's slightly easier to follow what's happening, even if it's slightly
less efficient. Code readability and maintainability is far, far more
important than micro-managed performance, in almost all cases.
> [[[
> 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'.
> ]]]
>
The patch looks okay to me, on first glance, but the detailed part of
log message is _far_ too detailed (and something appears to have eaten
all the leading spaces). Particularly, the 'note' is inappropriate for
the log message - if it should be mentioned anywhere (and I don't think
it should), it should be in the code. I'm also not sure what 'queries
of same kind' means.
Regards,
Malcolm
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Aug 31 12:32:06 2006