You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Lieven Govaerts <lg...@mobsol.be> on 2006/06/18 16:38:35 UTC

RE: [PATCH] Speedup the python tests (now with log message)

Same patch. 

Log message:
[[[
* subversion/tests/cmdline/svntest/actions.py
  (guarantee_greek_repository): checkout a pristine working copy.
  (make_repo_and_wc): instead of checking out the pristine repos, 
   make a copy of the pristine working copy and relocate it.

* subversion/tests/cmdline/svntest/main.py
  (pristine_wc_dir): new global  
  (copy_repos): when testing fsfs and test doesn't want a repos with 
   a unique UUID, do a tree copy of the pristine repos.
]]] 

> -----Original Message-----
> From: Lieven Govaerts [mailto:lgo@mobsol.be] 
> Sent: zondag 18 juni 2006 11:11
> To: dev@subversion.tigris.org
> Subject: [PATCH] Speedup the python tests
> 
> Hi,
> 
> 
> as discussed on IRC, I worked on a patch to improve the 
> performance of the python tests. Attached patch implements 
> two changes:
> 1. replace the svnadmin dump|svnadmin load method to create 
> the greek repository with a plain treecopy. This only when 
> testing fsfs and when the test doesn't require a new UUID for 
> the repo.
> 2. replace the checkout of the working copy per test with a 
> checkout once per file (eg. commit_tests.py) + a copy of that 
> wc to the sandbox + a switch --relocate of the copied wc.
> 
> All in all, I see on my Mac an improvement of 20% (run 
> completely of a RAM
> disk):
> make check: from 24'34s to 19'31s
> make svncheck: from 26'21s to 21'52s
> 
> I'd like to commit it, but want a second opinion first.
> 
> regards,
> 
> Lieven.
> 

RE: [PATCH] Speedup the python tests (now with log message)

Posted by Lieven Govaerts <lg...@mobsol.be>.
Philip Martin wrote:
> > Julian Foad <ju...@btopenworld.com> writes:
> > 
> >>OK, I don't dispute your evidence but I'm interested to know what the 
> >>definition of "live" is that applies to this greek tree reference 
> >>repository.
> > 
> > The usual: a repository is live when any process is accessing it.
> > 
> > In this case the problem was that the testsuite imported into, and 
> > then immediately copied, the repository.  The svnserve process that 
> > was forked to handle the import could still be running when the copy 
> > was made.
> 
I've updated the patch to use svnadmin hotcopy instead of a normal tree
copy. This is now the default for both FSFS and BDB repositories, as long as
the test doesn't require a repos with a new UUID. 

Performance is still similar as the treecopy version.

thanks for your input.

Lieven.

[[[
Improve the performance of the python tests:
 1. Create the test repos with svnadmin hotcopy instead of 
    svnadmin dump|svnadmin load.
 2. Check out a pristine working copy once, and copy+relocate it to
    the test working copy.

* subversion/tests/cmdline/svntest/actions.py
  (guarantee_greek_repository): checkout a pristine working copy.
  (make_repo_and_wc): instead of checking out the pristine repos, 
   make a copy of the pristine working copy and relocate it.

* subversion/tests/cmdline/svntest/main.py
  (pristine_wc_dir): new global
  (copy_repos): when a test doesn't want a repos with a unique UUID, 
   do a hotcopy of the pristine repos.
]]] 

Re: [PATCH] Speedup the python tests (now with log message)

Posted by Julian Foad <ju...@btopenworld.com>.
Philip Martin wrote:
> Julian Foad <ju...@btopenworld.com> writes:
> 
>>OK, I don't dispute your evidence but I'm interested to know what the
>>definition of "live" is that applies to this greek tree reference
>>repository.
> 
> The usual: a repository is live when any process is accessing it.
> 
> In this case the problem was that the testsuite imported into, and
> then immediately copied, the repository.  The svnserve process that
> was forked to handle the import could still be running when the copy
> was made.

Thanks.  That wasn't readily apparent to me but makes sense now.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Speedup the python tests (now with log message)

Posted by Philip Martin <ph...@codematters.co.uk>.
Julian Foad <ju...@btopenworld.com> writes:

> OK, I don't dispute your evidence but I'm interested to know what the
> definition of "live" is that applies to this greek tree reference
> repository.

The usual: a repository is live when any process is accessing it.

In this case the problem was that the testsuite imported into, and
then immediately copied, the repository.  The svnserve process that
was forked to handle the import could still be running when the copy
was made.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Speedup the python tests (now with log message)

Posted by Julian Foad <ju...@btopenworld.com>.
Philip Martin wrote:
> Julian Foad <ju...@btopenworld.com> writes:
>>
>>Use of "hotcopy" might require access to the BDB tools, but why can't
>>we use a straightforward directory tree copy?  We know that nothing
>>else is committing to this (greek tree) repository, don't we?  The
>>only possible simultaneous operation on it might be another set of
>>tests running in parallel in the same directory tree, which could be
>>doing "svnadmin dump" on it, but does that modify its files such that
>>we can't do a directory tree copy?
> 
> Using an OS copy on a live repository is not valid.  We used to do it
> and it led to errors when running the tests for BDB/ra_svn.

OK, I don't dispute your evidence but I'm interested to know what the 
definition of "live" is that applies to this greek tree reference repository.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Speedup the python tests (now with log message)

Posted by Philip Martin <ph...@codematters.co.uk>.
Julian Foad <ju...@btopenworld.com> writes:

> Lieven Govaerts wrote:
>>>Why only for FSFS?  It would be nice if the comment could give a hint.
>> Well, apparently long ago the choice was made to use svnadmin
>> dump|svnadmin
>> load for bdb, because that would require access to the BDB tools, so I
>> didn't want to touch that. I'll include a note in the log message.
>
> Use of "hotcopy" might require access to the BDB tools, but why can't
> we use a straightforward directory tree copy?  We know that nothing
> else is committing to this (greek tree) repository, don't we?  The
> only possible simultaneous operation on it might be another set of
> tests running in parallel in the same directory tree, which could be
> doing "svnadmin dump" on it, but does that modify its files such that
> we can't do a directory tree copy?

Using an OS copy on a live repository is not valid.  We used to do it
and it led to errors when running the tests for BDB/ra_svn.

An alternative to consider: add a greek tree dumpfile to the
Subversion repository and then simply load it straight into the
destination repository rather than creating/copying a separate
repository.  That might be a bit slower for the minority of tests that
use multiple repositories, but should be faster for the majority that
only use one repository.  It should work for both FSFS and BDB.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Speedup the python tests (now with log message)

Posted by Julian Foad <ju...@btopenworld.com>.
Lieven Govaerts wrote:
>>Why only for FSFS?  It would be nice if the comment could give a hint.
> 
> Well, apparently long ago the choice was made to use svnadmin dump|svnadmin
> load for bdb, because that would require access to the BDB tools, so I
> didn't want to touch that. I'll include a note in the log message.

Use of "hotcopy" might require access to the BDB tools, but why can't we use a 
straightforward directory tree copy?  We know that nothing else is committing 
to this (greek tree) repository, don't we?  The only possible simultaneous 
operation on it might be another set of tests running in parallel in the same 
directory tree, which could be doing "svnadmin dump" on it, but does that 
modify its files such that we can't do a directory tree copy?

>>This "fs_type is None" seems to be embedding knowlege of 
>>svnadmin's default, [...]
> 
> I was looking at that as well, you're right, I made the wrong choice here.
> I'll update it. Probably commit this separately from the speedup patch.

OK.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

RE: [PATCH] Speedup the python tests (now with log message)

Posted by Lieven Govaerts <lg...@mobsol.be>.
 

> -----Original Message-----
> From: Julian Foad [mailto:julianfoad@btopenworld.com] 
> Sent: zondag 18 juni 2006 21:44
> 
> Why only for FSFS?  It would be nice if the comment could give a hint.

Well, apparently long ago the choice was made to use svnadmin dump|svnadmin
load for bdb, because that would require access to the BDB tools, so I
didn't want to touch that. I'll include a note in the log message.

> >>+  if (fs_type == "fsfs" or fs_type is None) and not ignore_uuid:
> 
> This "fs_type is None" seems to be embedding knowlege of 
> svnadmin's default, which is not a good idea if it can be 
> avoided.  One way of avoiding it might be for this test suite 
> to decide on its own default (by initialising the global 
> variable "fs_type" to "fsfs") so that fs_type is never None.  
> That way this conditional would not be able to get out of 
> sync with the type of repository that gets created.  The test 
> suite's default repository type would then be FSFS even if 
> svnadmin's default were changed, but I don't see a problem in that.

I was looking at that as well, you're right, I made the wrong choice here.
I'll update it. Probably commit this separately from the speedup patch.
 
> Other than that, this looks fine and I'm happy with the 
> principle of copying repositories and WCs to speed up the test suite.
Ok, cool, thanks for the review.

Lieven.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Speedup the python tests (now with log message)

Posted by Julian Foad <ju...@btopenworld.com>.
Lieven Govaerts wrote:
>>Index: subversion/tests/cmdline/svntest/main.py
>>===================================================================
[...]
>>+  # If the repo is fsfs and the copy may have the same uuid, then just copy
>>+  # the repos files on disk.

Why only for FSFS?  It would be nice if the comment could give a hint.

>>+  if (fs_type == "fsfs" or fs_type is None) and not ignore_uuid:

This "fs_type is None" seems to be embedding knowlege of svnadmin's default, 
which is not a good idea if it can be avoided.  One way of avoiding it might be 
for this test suite to decide on its own default (by initialising the global 
variable "fs_type" to "fsfs") so that fs_type is never None.  That way this 
conditional would not be able to get out of sync with the type of repository 
that gets created.  The test suite's default repository type would then be FSFS 
even if svnadmin's default were changed, but I don't see a problem in that.

Other than that, this looks fine and I'm happy with the principle of copying 
repositories and WCs to speed up the test suite.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org