On Tue, Jun 14, 2011 at 5:45 PM, 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. >
I really hope this improves things. I haven't tested this myself on my Windows box, but from what I read it sound pretty serious. I think this is an important issue, because 'svn upgrade' will for most users be the first thing they experience with 1.7. It may be fine on *nix with local disk, but if it sucks on Windows ... it will really reflect badly on 1.7's performance/quality. Also, has anyone tested this on an NFS-working copy? Or CIFS? Maybe an upgrade test can be integrated in one or both of the perf-test-suites, and people can run them on some different platforms? Gut feeling: if the above wrap-in-transaction optimization helps out Windows/NTFS, chances are that NFS and CIFS working copies will also come to an acceptable level. IMHO, I'd say issue #3785 (improve performance of 'svn upgrade') is a 1.7 release-blocker. But maybe others have a different opinion about that. Anyway, if progress in this area is just around the corner, it probably doesn't matter that much :-). -- Johan
