> > Yes, INSERT OR IGNORE supported by all versions. I plan to start a separate > > thread about this topic. > > > > As mentioned above, I think we can optimize the insert statement for the > case of a constraint violation. Now we use the 'FAIL' conflict > resolution algorithm [1] and explicitly handle > SVN_ERR_SQLITE_CONSTRAINT error in svn_fs_fs__set_rep_reference(). > Because of this, we have an extra svn_fs_fs__get_rep_reference() call.
We do have an extra call when a duplicate rep is attempted to be inserted into the rep-cache. When does that happen? I know it will happen when called from build-repcache; what other use-cases trigger the codepath you changed? Committing two files with equal contents sequentially doesn't call svn_fs_fs__set_rep_reference() at all for the second file. > As far as I know, svn_sqlite__step() in svn_sqlite__insert() is the > only place where the error can occur. Based on this, I suggest to use > the 'IGNORE' conflict resolution algorithm [1] and remove the > SVN_ERR_SQLITE_CONSTRAINT handling, because the error should not occur > now. It seems to me the change is quite safe, because the behavior is > well covered by tests. > What tests is this behaviour covered by? > rep-cache.db insert optimization. > > Use the 'IGNORE' conflict resolution algorithm [1] for STMT_SET_REP > and remove SVN_ERR_SQLITE_CONSTRAINT handling, because the error > should not occur now. It brings a slight performance improvement, What scenario does this bring a performance improvement in? What is the improvement, quantitatively? > because we remove an extra svn_fs_fs__get_rep_reference() call. > > [1] https://www.sqlite.org/lang_conflict.html > > * subversion/libsvn_fs_fs/rep-cache-db.sql > (STMT_SET_REP): Use the 'IGNORE' conflict resolution algorithm. > > * subversion/libsvn_fs_fs/rep-cache.c > (svn_fs_fs__set_rep_reference): Remove SVN_ERR_SQLITE_CONSTRAINT handling. Cheers, Daniel
