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 09:10:43 UTC

[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

Posted by Lieven Govaerts <lg...@mobsol.be>.
> -----Original Message-----
> From: Julian Foad [mailto:julianfoad@btopenworld.com] 
> Sent: zondag 18 juni 2006 18:02

> Please could you post this patch with a formal log message.  
> That will make it easier to review.  The same goes for your 
> other patch today.
Sure, log messages submitted in my two previous mails, although I see I
forgot to include a general description of this patch; I'll include it
before I commit.
 
> > +      print 'CMD:', 'cp ' + src_path, ', ' , dst_path
> 
> Did you mean to print that comma between the paths?

The comma doesn't really make sense, I'll remove it.

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

Posted by Julian Foad <ju...@btopenworld.com>.
Please could you post this patch with a formal log message.  That will make it 
easier to review.  The same goes for your other patch today.

Lieven Govaerts wrote:
[...]
> +      print 'CMD:', 'cp ' + src_path, ', ' , dst_path

Did you mean to print that comma between the paths?

- 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

Posted by Ivan Zhakov <ch...@gmail.com>.
On 6/18/06, Lieven Govaerts <lg...@mobsol.be> wrote:
>
>
> > -----Original Message-----
> > From: Ivan Zhakov [mailto:chemodax@gmail.com]
> > Sent: zondag 18 juni 2006 20:40
>
> > Probably I missed something. Could you explain me why we
> > cannot use svnadmin hotcopy for copy repository? AFAIK it
> > works well with bdb and fsfs.
>
> No specific reason, except for the fact that there's a comment in main.py
> that says:
>     # A BDB hot-backup procedure would be more efficient, but that would
>     # require access to the BDB tools, and this doesn't.  Print a fake
>     # pipe command so that the displayed CMDs can be run by hand
>
> But that doesn't mean I can't use it for fsfs, thanks for the info.
>
This comment looks outdated -- at present time svnadmin hotcopy works
without BDB tools. For sure I've checked documentation for svnadmin
hotcopy [1], it doesn't depend on BDB tools. So I think you could try
to do it and wait if somebody complain :)

[1] http://svnbook.red-bean.com/en/1.0/re33.html

-- 
Ivan Zhakov

---------------------------------------------------------------------
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

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

> -----Original Message-----
> From: Ivan Zhakov [mailto:chemodax@gmail.com] 
> Sent: zondag 18 juni 2006 20:40

> Probably I missed something. Could you explain me why we 
> cannot use svnadmin hotcopy for copy repository? AFAIK it 
> works well with bdb and fsfs.

No specific reason, except for the fact that there's a comment in main.py
that says:
    # A BDB hot-backup procedure would be more efficient, but that would
    # require access to the BDB tools, and this doesn't.  Print a fake
    # pipe command so that the displayed CMDs can be run by hand

But that doesn't mean I can't use it for fsfs, thanks for the info.

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

Posted by Ivan Zhakov <ch...@gmail.com>.
On 6/18/06, Lieven Govaerts <lg...@mobsol.be> wrote:
> 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.
Probably I missed something. Could you explain me why we cannot use
svnadmin hotcopy for copy repository? AFAIK it works well with bdb and
fsfs.

[...]



-- 
Ivan Zhakov

---------------------------------------------------------------------
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>.
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

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

Posted by Lieven Govaerts <lg...@mobsol.be>.
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.
>