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 2010/09/03 19:34:52 UTC
svn commit: r992390 - in /subversion/trunk/subversion:
include/private/svn_sqlite.h libsvn_subr/sqlite.c libsvn_wc/wc_db.c
libsvn_wc/wc_db.h
Author: rhuijben
Date: Fri Sep 3 17:34:52 2010
New Revision: 992390
URL: http://svn.apache.org/viewvc?rev=992390&view=rev
Log:
Add a few helper functions to allow experimenting with SQLite operations
inside SAVEPOINTs. This allows taking out a shared read lock for longer than
the duration of a single SQLite statement, which gives a huge performance
boost in some hacky tests. (Especially with SQLite 3.7 and on Windows, where
some of the WAL code slows down SQLite more than it used to do in 3.6)
* subversion/include/private/svn_sqlite.h
(svn_sqlite__with_lock): New function.
* subversion/libsvn_subr/sqlite.c
(svn_sqlite__with_lock): New function.
* subversion/libsvn_wc/wc_db.c
(with_sqlite_lock_baton): New struct.
(call_sqlite_lock_cb): New function.
(svn_wc__db_with_sqlite_lock): New function.
* subversion/libsvn_wc/wc_db.h
(svn_wc__db_with_sqlite_lock): New function.
Modified:
subversion/trunk/subversion/include/private/svn_sqlite.h
subversion/trunk/subversion/libsvn_subr/sqlite.c
subversion/trunk/subversion/libsvn_wc/wc_db.c
subversion/trunk/subversion/libsvn_wc/wc_db.h
Modified: subversion/trunk/subversion/include/private/svn_sqlite.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_sqlite.h?rev=992390&r1=992389&r2=992390&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_sqlite.h (original)
+++ subversion/trunk/subversion/include/private/svn_sqlite.h Fri Sep 3 17:34:52 2010
@@ -310,6 +310,34 @@ svn_sqlite__with_transaction(svn_sqlite_
void *cb_baton, apr_pool_t *scratch_pool);
+/* Helper function to handle several SQLite operations inside a shared lock.
+ This callback is similar to svn_sqlite__with_transaction(), but can be
+ nested (even with a transaction) and changes in the callback are always
+ committed when this function returns.
+
+ Behavior on an application crash while this function is running is
+ UNDEFINED: Either everything is committed (for < 3.6.8) or is not (for
+ >= 3.6.8 where this function uses a SAVEPOINT), so this should only be used
+ for operations that are safe under these conditions or just for reading.
+
+ Use a transaction when you need explicit behavior.
+
+ For SQLite 3.6.8 and later using this function as a wrapper around a group
+ of operations can give a *huge* performance boost as the shared-read lock
+ will be shared over multiple statements, instead of being reobtained
+ everytime, which requires disk and/or network io.
+
+ ### It might be possible to implement the same lock behavior for < 3.6.8
+ by keeping a read SQLite statement open, but this wouldn't replicate
+ the rollback behavior on crashing. Maybe we should just require 3.6.8?
+ */
+svn_error_t *
+svn_sqlite__with_lock(svn_sqlite__db_t *db,
+ svn_sqlite__transaction_callback_t cb_func,
+ void *cb_baton,
+ apr_pool_t *scratch_pool);
+
+
/* Hotcopy an SQLite database from SRC_PATH to DST_PATH. */
svn_error_t *
svn_sqlite__hotcopy(const char *src_path,
Modified: subversion/trunk/subversion/libsvn_subr/sqlite.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/sqlite.c?rev=992390&r1=992389&r2=992390&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/sqlite.c (original)
+++ subversion/trunk/subversion/libsvn_subr/sqlite.c Fri Sep 3 17:34:52 2010
@@ -71,6 +71,7 @@ struct svn_sqlite__db_t
int nbr_statements;
svn_sqlite__stmt_t **prepared_stmts;
apr_pool_t *state_pool;
+ unsigned savepoint_nr;
};
struct svn_sqlite__stmt_t
@@ -1018,6 +1019,60 @@ svn_sqlite__with_transaction(svn_sqlite_
}
svn_error_t *
+svn_sqlite__with_lock(svn_sqlite__db_t *db,
+ svn_sqlite__transaction_callback_t cb_func,
+ void *cb_baton,
+ apr_pool_t *scratch_pool)
+{
+ svn_error_t *err;
+
+#if SQLITE_VERSION_AT_LEAST(3,6,8)
+ svn_error_t *err2;
+ int savepoint = db->savepoint_nr++;
+ const char *release_stmt;
+
+ SVN_ERR(exec_sql(db,
+ apr_psprintf(scratch_pool, "SAVEPOINT s%u;", savepoint)));
+#endif
+
+ err = cb_func(cb_baton, db, scratch_pool);
+
+#if SQLITE_VERSION_AT_LEAST(3,6,8)
+ release_stmt = apr_psprintf(scratch_pool, "RELEASE s%u;", savepoint);
+ err2 = exec_sql(db, release_stmt);
+
+ if (err2 && err2->apr_err == SVN_ERR_SQLITE_BUSY)
+ {
+ /* Ok, we have a major problem. Some statement is still open, which
+ makes it impossible to release this savepoint.
+
+ ### See huge comment in svn_sqlite__with_transaction for
+ further details */
+
+ int i;
+
+ err2 = svn_error_compose_create(err2,
+ svn_error_create(SVN_ERR_SQLITE_RESETTING_FOR_ROLLBACK,
+ NULL, NULL));
+
+ for (i = 0; i < db->nbr_statements; i++)
+ if (db->prepared_stmts[i] && db->prepared_stmts[i]->needs_reset)
+ err2 = svn_error_compose_create(
+ err2,
+ svn_sqlite__reset(db->prepared_stmts[i]));
+
+ err2 = svn_error_compose_create(
+ exec_sql(db, release_stmt),
+ err2);
+ }
+
+ err = svn_error_compose_create(err, err2);
+#endif
+
+ return svn_error_return(err);
+}
+
+svn_error_t *
svn_sqlite__hotcopy(const char *src_path,
const char *dst_path,
apr_pool_t *scratch_pool)
Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=992390&r1=992389&r2=992390&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Fri Sep 3 17:34:52 2010
@@ -1583,6 +1583,58 @@ svn_wc__db_get_wcroot(const char **wcroo
return SVN_NO_ERROR;
}
+struct with_sqlite_lock_baton
+{
+ svn_wc__db_t *db;
+ svn_wc__db_sqlite_lock_cb lock_cb;
+ void *lock_baton;
+};
+
+static svn_error_t *
+call_sqlite_lock_cb(void *baton,
+ svn_sqlite__db_t *sdb,
+ apr_pool_t *scratch_pool)
+{
+ struct with_sqlite_lock_baton *lb = baton;
+
+ return svn_error_return(lb->lock_cb(lb->db, lb->lock_baton, scratch_pool));
+}
+
+svn_error_t *
+svn_wc__db_with_sqlite_lock(svn_wc__db_t *db,
+ const char *wri_abspath,
+ svn_wc__db_sqlite_lock_cb lock_cb,
+ void *cb_baton,
+ apr_pool_t *scratch_pool)
+{
+ svn_wc__db_pdh_t *pdh;
+ const char *unused_relpath;
+ struct with_sqlite_lock_baton baton;
+
+ SVN_ERR(svn_wc__db_pdh_parse_local_abspath(&pdh, &unused_relpath, db,
+ wri_abspath, svn_sqlite__mode_readonly,
+ scratch_pool, scratch_pool));
+
+ /* Can't use VERIFY_USABLE_PDH, as this should be usable to detect
+ where call upgrade */
+
+ if (pdh->wcroot == NULL || !pdh->wcroot->sdb)
+ return svn_error_createf(SVN_ERR_WC_NOT_WORKING_COPY, NULL,
+ _("The node '%s' is not in a workingcopy."),
+ svn_dirent_local_style(wri_abspath,
+ scratch_pool));
+
+ baton.db = db;
+ baton.lock_cb = lock_cb;
+ baton.lock_baton = cb_baton;
+
+ return svn_error_return(
+ svn_sqlite__with_lock(pdh->wcroot->sdb,
+ call_sqlite_lock_cb,
+ &baton,
+ scratch_pool));
+}
+
svn_error_t *
svn_wc__db_base_add_directory(svn_wc__db_t *db,
const char *local_abspath,
Modified: subversion/trunk/subversion/libsvn_wc/wc_db.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.h?rev=992390&r1=992389&r2=992390&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.h (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.h Fri Sep 3 17:34:52 2010
@@ -428,6 +428,25 @@ svn_wc__db_get_wcroot(const char **wcroo
apr_pool_t *result_pool,
apr_pool_t *scratch_pool);
+typedef svn_error_t * (*svn_wc__db_sqlite_lock_cb)(svn_wc__db_t *db,
+ void *baton,
+ apr_pool_t *scratch_pool);
+
+/* Obtain a sqlite lock to efficiently use the working copy database for
+ WRI_ABSPATH and call LOCK_CB with CB_BATON while this lock exist.
+
+ On returning from this function all operations will be committed, but
+ operations might have been committed before returning from this function.
+
+ ### See svn_sqlite__with_lock() for more details.
+ */
+svn_error_t *
+svn_wc__db_with_sqlite_lock(svn_wc__db_t *db,
+ const char *wri_abspath,
+ svn_wc__db_sqlite_lock_cb lock_cb,
+ void *cb_baton,
+ apr_pool_t *scratch_pool);
+
/* @} */
/* Different kinds of trees
Re: svn commit: r992390 - in /subversion/trunk/subversion:
include/private/svn_sqlite.h libsvn_subr/sqlite.c libsvn_wc/wc_db.c
libsvn_wc/wc_db.h
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
rhuijben@apache.org wrote on Fri, Sep 03, 2010 at 17:34:52 -0000:
> + for (i = 0; i < db->nbr_statements; i++)
> + if (db->prepared_stmts[i] && db->prepared_stmts[i]->needs_reset)
> + err2 = svn_error_compose_create(
> + err2,
> + svn_sqlite__reset(db->prepared_stmts[i]));
> +
> + err2 = svn_error_compose_create(
> + exec_sql(db, release_stmt),
> + err2);
Should the second "err2 = ..." line be inside the if, or not?
If yes, need { } block.
If not, indentation is wrong.
Re: svn commit: r992390 - in /subversion/trunk/subversion:
include/private/svn_sqlite.h libsvn_subr/sqlite.c libsvn_wc/wc_db.c libsvn_wc/wc_db.h
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Tue, Sep 7, 2010 at 1:08 PM, Greg Stein <gs...@gmail.com> wrote:
> On Fri, Sep 3, 2010 at 13:34, <rh...@apache.org> wrote:
>> Author: rhuijben
>> Date: Fri Sep 3 17:34:52 2010
>> New Revision: 992390
>>
>> URL: http://svn.apache.org/viewvc?rev=992390&view=rev
>> Log:
>> Add a few helper functions to allow experimenting with SQLite operations
>> inside SAVEPOINTs. This allows taking out a shared read lock for longer than
>> the duration of a single SQLite statement, which gives a huge performance
>> boost in some hacky tests. (Especially with SQLite 3.7 and on Windows, where
>> some of the WAL code slows down SQLite more than it used to do in 3.6)
>>
>> * subversion/include/private/svn_sqlite.h
>> (svn_sqlite__with_lock): New function.
>>
>> * subversion/libsvn_subr/sqlite.c
>> (svn_sqlite__with_lock): New function.
>>
>> * subversion/libsvn_wc/wc_db.c
>> (with_sqlite_lock_baton): New struct.
>> (call_sqlite_lock_cb): New function.
>> (svn_wc__db_with_sqlite_lock): New function.
>>
>> * subversion/libsvn_wc/wc_db.h
>> (svn_wc__db_with_sqlite_lock): New function.
>
> Why is this part of the WC_DB interface?!? I see no reason to expose
> sqlite to callers of the DB. Let alone mechanics such as locking...
+1
-Hyrum
Re: svn commit: r992390 - in /subversion/trunk/subversion:
include/private/svn_sqlite.h libsvn_subr/sqlite.c libsvn_wc/wc_db.c libsvn_wc/wc_db.h
Posted by Greg Stein <gs...@gmail.com>.
On Fri, Sep 3, 2010 at 13:34, <rh...@apache.org> wrote:
> Author: rhuijben
> Date: Fri Sep 3 17:34:52 2010
> New Revision: 992390
>
> URL: http://svn.apache.org/viewvc?rev=992390&view=rev
> Log:
> Add a few helper functions to allow experimenting with SQLite operations
> inside SAVEPOINTs. This allows taking out a shared read lock for longer than
> the duration of a single SQLite statement, which gives a huge performance
> boost in some hacky tests. (Especially with SQLite 3.7 and on Windows, where
> some of the WAL code slows down SQLite more than it used to do in 3.6)
>
> * subversion/include/private/svn_sqlite.h
> (svn_sqlite__with_lock): New function.
>
> * subversion/libsvn_subr/sqlite.c
> (svn_sqlite__with_lock): New function.
>
> * subversion/libsvn_wc/wc_db.c
> (with_sqlite_lock_baton): New struct.
> (call_sqlite_lock_cb): New function.
> (svn_wc__db_with_sqlite_lock): New function.
>
> * subversion/libsvn_wc/wc_db.h
> (svn_wc__db_with_sqlite_lock): New function.
Why is this part of the WC_DB interface?!? I see no reason to expose
sqlite to callers of the DB. Let alone mechanics such as locking...
Cheers,
-g
Re: svn commit: r992390 - in /subversion/trunk/subversion:
include/private/svn_sqlite.h libsvn_subr/sqlite.c libsvn_wc/wc_db.c
libsvn_wc/wc_db.h
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
rhuijben@apache.org wrote on Fri, Sep 03, 2010 at 17:34:52 -0000:
> + for (i = 0; i < db->nbr_statements; i++)
> + if (db->prepared_stmts[i] && db->prepared_stmts[i]->needs_reset)
> + err2 = svn_error_compose_create(
> + err2,
> + svn_sqlite__reset(db->prepared_stmts[i]));
> +
> + err2 = svn_error_compose_create(
> + exec_sql(db, release_stmt),
> + err2);
Should the second "err2 = ..." line be inside the if, or not?
If yes, need { } block.
If not, indentation is wrong.