On Tue, Jun 14, 2011 at 6:47 PM, Paul Burba <ptbu...@gmail.com> wrote: > On Tue, Jun 14, 2011 at 11:45 AM, Paul Burba <ptbu...@gmail.com> wrote: >> On Tue, Jun 14, 2011 at 3:41 AM, Philip Martin >> <philip.mar...@wandisco.com> wrote: >>> Paul Burba <ptbu...@gmail.com> writes: >>> >>>> No surprise that your checkouts are faster than mine given you are >>>> using a local mirror. What's puzzling me is why my upgrades are so >>>> much slower than yours. >>>> >>>> Running an upgrade of a trunk WC on my machine under xperf takes >>>> 00:03:46.351 elapsed and 11.44s CPU time using my primary drive (320 >>>> GB, SATA-II, 7200 rpm, 16 MB Cache, NTFS). Subversion spends 50s >>>> total disk service time (46.8s of that is read service time). >>> >>> Does "total disk service time" represent all the time waiting for disk >>> IO? 11s CPU plus 50s disk still leaves 165s unaccounted. >> >> I've only just started using xperf (it's part of the Windows >> Performance Toolkit) and can't find an exact definition of their >> terminology. I can say that xperf adds lot of overhead since upgrades >> run under it reliably take about twice as long. >> >> The 165s of unaccounted time is largely made up of 143s of disk >> service time by the system process...but now that I dig a little >> further I find that the system process' disk utilization is touching >> every directory in the working copy as well as writing the new >> .svn\pristine\ stuffs. Previously I was only looking at the disk >> utilization by the svn.exe process itself. >> >> ~~~~~ >> >> Not sure what to make of this: I tried an upgrade of a trunk WC using >> trunk@1135642 on my HDD and then my SSD (on the same box) >> >> Elapsed time for the HDD: 00:01:12.450 >> For the SSD: 00:00:09.267 >> >> Not sure if this points to anything other than SSDs are faster (duh!), >> but the degree of improvement was a unexpected. >> >>> Upgrade >>> prints a notification line for each directory, is there a significant >>> delay before the first line, or after the last line, or is the delay >>> spread among the lines? >> >> In all my testing I used the --quiet option. I just now tried it >> without and the delay is definitely spread among the output. >> >>> If it is a Subversion problem rather than your machine there are two >>> areas that may be worth investigating (but it's hard to say when most of >>> the time is unaccounted). >>> >>> The property migration currently invokes multiple transactions >>> per-directory. Moving to a single transaction per-directory will >>> probably help. >>> >>> Upgrade currently copies all the text-bases, I did experiment with >>> creating workqueue items to move them instead but it wasn't any faster >>> on my Linux box. However it may help on Windows. >> >> A bit of cut-and-paste from IRC this morning for the benefit of others >> reading this thread: >> >> <philipm> Bert, pburba: I suppose we could do the whole upgrade as a >> single transaction. >> <philipm> The database will never be accessed by more than one >> thread/process during upgrade. >> <philipm> And "partial" upgrades are already thrown away. >> >> <Bert> philipm: Agree >> >> <hwright> philipm: we have nested txns in sqlite, so you should be >> able to just wrap the entire upgrade process in a txn >> >> <Bert> philipm: Last week I tried to reduce the number of stats in the >> property load code by just fetching all the entries in the property >> dirs, but the perf difference was not measurable. I think making it a >> single transaction would have the most impact. >> >> <pburba> philipm, Bert, hwright : Have we done anything similar to >> that already (wrapping the whole upgrade in a single transaction)? I'd >> like to tackle it, but looking at similar work to provide some >> understanding/traction would be very helpful >> >> <hwright> pburba: we don't expose txns outside of wc_db >> >> <philipm> pburba: What you want is upgrade_working_copy as a callback >> from svn_wc__db_with_txn >> >> <hwright> I'm not sure how much of the work is done in wc_db (and how >> much without), but that's the first thing to consider >> >> <philipm> We would have to move lots of code, or expose the txn to >> upgrade.c in some form. >> >> <hwright> upgrade is such a special case, it makes sense to >> potentially expose it in a limited fashion >> >> <Bert> svn_sqlite__with_lock() >> >> <philipm> upgrade.c already access the sqlite db directly >> >> <hwright> philipm: in that case, just do the txn in upgrade.c, and >> wc_db.c should be fine due to txn nesting, yes? >> >> <philipm> Yes. Putting svn_sqlite__with_lock around >> upgrade_working_copy in upgrade.c will probably be OK >> >> <hwright> (if it isn't, you'll know pretty quick. :) ) >> >> <philipm> Upgrade uses a txn to write entries, per-dir. >> <philipm> But if the whole upgrade is a txn that could be removed, >> it's not used for anything other than upgrade. >> <philipm> With nested txns that will not be necessary, but avoiding >> nested txns may have a performance benefit? >> <philipm> I don't know. For a first attempt just put >> upgrade_working_copy in svn_sqlite__with_lock. >> >> <pburba> philipm: Ok, I'll give that a try then. > > Here's a stab at it. In my ad-hoc testing, the attached patch > improved the elapsed upgrade time of a ^/subversion/trunk WC from > 00:02:00 to 00:01:34 (3 runs). It improved an upgrade of a > ^/subversion/tags/ebcdic WC from 00:11:44 to 00:06:52 (single run).
Committed r1136525 > [[[ > Improve 'svn upgrade' performance by moving more of the upgrade process into > a single sqlite transaction. > > * subversion/libsvn_wc/entries.c > > (entries_write_baton, > entries_write_new_cb): Remove. > > (svn_wc__write_upgraded_entries): Don't run this part of the upgrade > operation in a dedicated sqlite transaction, but rather embed it in a > larger transaction. Also use the existing iterpool when creating > tmp_entry_abspath argument to write_entry. > > * subversion/libsvn_wc/upgrade.c > > (upgrade_to_wcng): Don't create the new DB here, move that to svn_wc_upgrade. > > (upgrade_working_copy_baton_t, > upgrade_working_copy_txn): New baton and transaction wrapper for > upgrade_working_copy. > > (svn_wc_upgrade): Create the new wcng DB here and wrap the call to > upgrade_working_copy() in a sqlite transaction. > ]]] > > Paul >
