[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Fresh checkout vs 'svn upgrade': How good is good enough?

From: Paul Burba <ptburba_at_gmail.com>
Date: Thu, 16 Jun 2011 12:20:10 -0400

On Tue, Jun 14, 2011 at 6:47 PM, Paul Burba <ptburba_at_gmail.com> wrote:
> On Tue, Jun 14, 2011 at 11:45 AM, Paul Burba <ptburba_at_gmail.com> wrote:
>> On Tue, Jun 14, 2011 at 3:41 AM, Philip Martin
>> <philip.martin_at_wandisco.com> wrote:
>>> Paul Burba <ptburba_at_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_at_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
>
Received on 2011-06-16 18:20:42 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.