You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Erik Huelsmann <eh...@gmail.com> on 2007/03/11 13:12:25 UTC
[RFC] Stop sleeping for timestamps when running tests
In the last few days, I committed an extra criterion to help determine
'changedness' of a file: libsvn_wc also considers the size of the file
at hand. So, now we do not only depend on the timestamp any more. If a
file changes in size (but not in time), svn will detect it's possibly
changed.
In our test-suite all but 2 tests change the sizes of the files
involved in the test. That means that all (except these 2) tests don't
have to depend on the timestamp to complete successfully.
I added (locally) a test to svn_sleep_for_timestamps() to check an
environment variable whether or not to sleep for stamps. When running
the test suite with that variable set to the right value,
basic_tests.py took 24seconds.
Recently lgo added parellel (?) execution of tests. Tests run in
parellel also take 24seconds.
I'd like to change our test suite to not sleep for timestamps except
for a specified number of tests. Currently the specified tests would
be the ones which fail without sleeping. That way, we won't need the
parellelized feature (lgo prefers to rip that).
Additional benefit of not-sleeping versus parellelizing is that the
on-disk space-requirements stay the same as they currently are. This
could be important for execution on RAM disks.
Comments?
bye,
Erik
The patch to libsvn_subr:svn_sleep_for_timestamps():
Index: subversion/libsvn_subr/time.c
===================================================================
--- subversion/libsvn_subr/time.c (revision 23768)
+++ subversion/libsvn_subr/time.c (working copy)
@@ -2,7 +2,7 @@
* time.c: time/date utilities
*
* ====================================================================
- * Copyright (c) 2000-2004 CollabNet. All rights reserved.
+ * Copyright (c) 2000-2007 CollabNet. All rights reserved.
*
* This software is licensed as described in the file COPYING, which
* you should have received as part of this distribution. The terms
@@ -289,9 +289,13 @@
svn_sleep_for_timestamps(void)
{
apr_time_t now, then;
+ const char *env_val = getenv("SVN_SLEEP_FOR_TIMESTAMPS");
/* Sleep until the next second tick, plus a tenth of a second for margin. */
- now = apr_time_now();
- then = apr_time_make(apr_time_sec(now) + 1, APR_USEC_PER_SEC / 10);
- apr_sleep(then - now);
+ if (apr_strnatcasecmp(env_val, "no") != 0)
+ {
+ now = apr_time_now();
+ then = apr_time_make(apr_time_sec(now) + 1, APR_USEC_PER_SEC / 10);
+ apr_sleep(then - now);
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [RFC] Stop sleeping for timestamps when running tests
Posted by Ben Collins-Sussman <su...@red-bean.com>.
On 3/12/07, Dan Christian <dc...@google.com> wrote:
> What would it take to reduce/remove the sleeps all the time?
>
> Just the other day I needed to build up a directory structure for testing.
> I wrapped up a quick script that did "svn mkdir http://..." for each one
> (this is over WebDAV). The performance was horrible! Each one took 2-3
> seconds.
I think this example isn't relevant -- it doesn't use the working copy
at all, and all of this sleep() discussion is about working copy
bookkeeping. In your example above, 'svn mkdir URL' is slow because
WebDAV is a horrible horrible protocol with at least a dozen
turnarounds.
That said, I think your point still stands in general. :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [RFC] Stop sleeping for timestamps when running tests
Posted by Dan Christian <dc...@google.com>.
What would it take to reduce/remove the sleeps all the time?
Just the other day I needed to build up a directory structure for testing.
I wrapped up a quick script that did "svn mkdir http://..." for each one
(this is over WebDAV). The performance was horrible! Each one took 2-3
seconds.
The point is that many simple operations are sluggish because of this "wait
for next second" stuff. The sleep dominates the transaction time and make
svn appear much slower than it could be. I'm trying to sell this to me
team, but I have to dance around the fact that some simple operations seem
to be really slow (while "hard" things like "diff" are quite fast).
Can we kill the sleeps without losing correctness?
I've got CPU and memory to spare, but I can never get back the time.
-Dan C
P.S. Yes, there are other ways to do this that would be faster. That's not
the point. I actually rewrote the script twice while waiting for the
original implementation to finish. That kind of "gotcha" doesn't help me
promote svn.
P.P.S. I also don't know why it is doing the sleep when there is no WC
involved. I'm guessing it is following the same code path for both the pure
URL case and the WC case.
On 3/11/07, Erik Huelsmann <eh...@gmail.com> wrote:
>
> In the last few days, I committed an extra criterion to help determine
> 'changedness' of a file: libsvn_wc also considers the size of the file
> at hand. So, now we do not only depend on the timestamp any more. If a
> file changes in size (but not in time), svn will detect it's possibly
> changed.
>
>
Re: [RFC] Stop sleeping for timestamps when running tests
Posted by Erik Huelsmann <eh...@gmail.com>.
On 3/12/07, Peter Lundblad <pl...@google.com> wrote:
> Ivan Zhakov writes:
> > - I propose to make semantic of variable similar as SVN_ASP_DOT_NET_HACK.
> > I.e. make variable SVN_NO_SLEEP_FOR_TIMESTAMPS and doesn't care
> > about it's contents.
> >
> >
>
> Erik,
>
> You're talking about running tests, but your patch unconditinally checks for
> this variable.
Yes. Because I thought about disabling the sleep in maintainer-mode
builds, but it turned out to be a bad idea. Then, several iterations
later, I had a 'cleaner' code instance (the current one), which didn't
really need conditionalising, because I thought it was 'clean
enough'...
> - When sending out an RFC, it is often good to wait a few days to collect
> feedback. Why rush with this patch?
Well, I'm not trying to rush the patch itself as much as that I'm
trying to clear my working copy again: in the past I've had other
cases with un-committed stuff in my working copy which has ended up in
commits of other stuff. I'd like to have a clean slate. I'll remove it
again as soon as there are objections.
> - You're adding this functionality to a (more or less) general-purpose
> A:I, but the documentation says nothing about that.
> - The implementation makes this available in all builds.
> "Don't tell anyone about it, but there's an undocumented hack that makes
> your client slightly faster." Is this something we want to support?
In cases where the sleep actually matters (ie slows down the process),
we're probably looking at automated environments. Exactly these
environments require the sleep. So, generally, no, I don't think we
want users to use this feature. Though I'm not really 'hiding' it or
anything.
> - This makes us not run the code people use when we run our tests.
> I know the slow test suite is annoying, but I'm against circumventing the
> sleep when running most tests by default.
Only a very limited number of users needs these sleeps. Any operation
which optionally writes to the working copy needs sleeps to work well
in an automated environment.
I think we can quite well estimate which operations should require
sleeps and test their behaviour in the test suite.
What I mean to say with the above remarks is that
1) we can explicitly test for those cases
2) the importance of sleeping has reduced (but not eliminated) with
the working-size change
While I typed the above, we had a discussion on IRC which I'll try to summarize:
1) users may not need it, but generally when it hurts the most, they
do (because the sleep is designed to make timestamps a reliable
heuristic especially in automated environments)
2) the current code isn't conditionalized on maintainer-mode, but it
should be, because otherwise we'll unwillingly export an option to
users we don't want them to use.
3) the envvar would also (next to [2]) be inconsistent with other
means of providing user options (because those are provided through
the config file)
Which means that we concluded the code consulting the envvar isn't
wrong, but should be conditionalized.
bye,
Erik.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [RFC] Stop sleeping for timestamps when running tests
Posted by Lieven Govaerts <sv...@mobsol.be>.
Quoting Peter Lundblad <pl...@google.com>:
> - This makes us not run the code people use when we run our tests.
> I know the slow test suite is annoying, but I'm against circumventing the
> sleep when running most tests by default.
There still are some tests that are run with sleep because they don't change the
file size. Part of this change is (discussed with erik, but not implemented yet)
to add a new test that explicitly tests the sleep behavior.
One test covering the sleep behavior is sufficient, we don't need to test the
same code 300 times.
Lieven
----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [RFC] Stop sleeping for timestamps when running tests
Posted by Peter Lundblad <pl...@google.com>.
Ivan Zhakov writes:
> - I propose to make semantic of variable similar as SVN_ASP_DOT_NET_HACK.
> I.e. make variable SVN_NO_SLEEP_FOR_TIMESTAMPS and doesn't care
> about it's contents.
>
>
Erik,
You're talking about running tests, but your patch unconditinally checks for
this variable.
- When sending out an RFC, it is often good to wait a few days to collect
feedback. Why rush with this patch?
- You're adding this functionality to a (more or less) general-purpose
A:I, but the documentation says nothing about that.
- The implementation makes this available in all builds.
"Don't tell anyone about it, but there's an undocumented hack that makes
your client slightly faster." Is this something we want to support?
- This makes us not run the code people use when we run our tests.
I know the slow test suite is annoying, but I'm against circumventing the
sleep when running most tests by default.
Thanks,
//Peter
> On 3/11/07, Erik Huelsmann <eh...@gmail.com> wrote:
> > In the last few days, I committed an extra criterion to help determine
> > 'changedness' of a file: libsvn_wc also considers the size of the file
> > at hand. So, now we do not only depend on the timestamp any more. If a
> > file changes in size (but not in time), svn will detect it's possibly
> > changed.
> >
> > In our test-suite all but 2 tests change the sizes of the files
> > involved in the test. That means that all (except these 2) tests don't
> > have to depend on the timestamp to complete successfully.
> >
> > I added (locally) a test to svn_sleep_for_timestamps() to check an
> > environment variable whether or not to sleep for stamps. When running
> > the test suite with that variable set to the right value,
> > basic_tests.py took 24seconds.
> >
> > Recently lgo added parellel (?) execution of tests. Tests run in
> > parellel also take 24seconds.
> >
> > I'd like to change our test suite to not sleep for timestamps except
> > for a specified number of tests. Currently the specified tests would
> > be the ones which fail without sleeping. That way, we won't need the
> > parellelized feature (lgo prefers to rip that).
> >
> > Additional benefit of not-sleeping versus parellelizing is that the
> > on-disk space-requirements stay the same as they currently are. This
> > could be important for execution on RAM disks.
> >
> >
> > Comments?
> >
> >
> > bye,
> >
> > Erik
> >
> > The patch to libsvn_subr:svn_sleep_for_timestamps():
> >
> > Index: subversion/libsvn_subr/time.c
> > ===================================================================
> > --- subversion/libsvn_subr/time.c (revision 23768)
> > +++ subversion/libsvn_subr/time.c (working copy)
> > @@ -2,7 +2,7 @@
> > * time.c: time/date utilities
> > *
> > * ====================================================================
> > - * Copyright (c) 2000-2004 CollabNet. All rights reserved.
> > + * Copyright (c) 2000-2007 CollabNet. All rights reserved.
> > *
> > * This software is licensed as described in the file COPYING, which
> > * you should have received as part of this distribution. The terms
> > @@ -289,9 +289,13 @@
> > svn_sleep_for_timestamps(void)
> > {
> > apr_time_t now, then;
> > + const char *env_val = getenv("SVN_SLEEP_FOR_TIMESTAMPS");
> >
> > /* Sleep until the next second tick, plus a tenth of a second for margin. */
> > - now = apr_time_now();
> > - then = apr_time_make(apr_time_sec(now) + 1, APR_USEC_PER_SEC / 10);
> > - apr_sleep(then - now);
> > + if (apr_strnatcasecmp(env_val, "no") != 0)
> > + {
> > + now = apr_time_now();
> > + then = apr_time_make(apr_time_sec(now) + 1, APR_USEC_PER_SEC / 10);
> > + apr_sleep(then - now);
> > + }
> > }
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> > For additional commands, e-mail: dev-help@subversion.tigris.org
> >
> >
>
>
> --
> Ivan Zhakov
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [RFC] Stop sleeping for timestamps when running tests
Posted by Dan Christian <dc...@google.com>.
On 3/12/07, Erik Huelsmann <eh...@gmail.com> wrote:
> On 3/12/07, Ivan Zhakov <ch...@gmail.com> wrote:
> > Hi Erik,
> >
> > I've no thoughts about idea itself, but I've several comments about
> > implementation.
> > - According documentation getenv() returns NULL if environment variable didn't
> > set. You don't check this.
>
> I do now. Yes, this made my client segfault before I noticed.
>
> > - I propose to make semantic of variable similar as SVN_ASP_DOT_NET_HACK.
> > I.e. make variable SVN_NO_SLEEP_FOR_TIMESTAMPS and doesn't care
> > about it's contents.
>
> Well, it is easy to set the value to 'yes' or 'no' from python. I
> haven't studied how to delete the variable from the environment
> entirely (from python). That's why I took the value into account.
If you "del" the proper python environment entry, it will get deleted
from the process environment.
Having said that. If this code is always enabled, then someone will
find and abuse it. It should only be enabled if the behavior is
correct in all cases.
It should be explicitly enabled at compile time and indicate that it
was built in maintainer mode (maybe in the version string).
-Dan C
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [RFC] Stop sleeping for timestamps when running tests
Posted by Erik Huelsmann <eh...@gmail.com>.
On 3/12/07, Ivan Zhakov <ch...@gmail.com> wrote:
> Hi Erik,
>
> I've no thoughts about idea itself, but I've several comments about
> implementation.
> - According documentation getenv() returns NULL if environment variable didn't
> set. You don't check this.
I do now. Yes, this made my client segfault before I noticed.
> - I propose to make semantic of variable similar as SVN_ASP_DOT_NET_HACK.
> I.e. make variable SVN_NO_SLEEP_FOR_TIMESTAMPS and doesn't care
> about it's contents.
Well, it is easy to set the value to 'yes' or 'no' from python. I
haven't studied how to delete the variable from the environment
entirely (from python). That's why I took the value into account.
bye,
Erik.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [RFC] Stop sleeping for timestamps when running tests
Posted by Ivan Zhakov <ch...@gmail.com>.
Hi Erik,
I've no thoughts about idea itself, but I've several comments about
implementation.
- According documentation getenv() returns NULL if environment variable didn't
set. You don't check this.
- I propose to make semantic of variable similar as SVN_ASP_DOT_NET_HACK.
I.e. make variable SVN_NO_SLEEP_FOR_TIMESTAMPS and doesn't care
about it's contents.
On 3/11/07, Erik Huelsmann <eh...@gmail.com> wrote:
> In the last few days, I committed an extra criterion to help determine
> 'changedness' of a file: libsvn_wc also considers the size of the file
> at hand. So, now we do not only depend on the timestamp any more. If a
> file changes in size (but not in time), svn will detect it's possibly
> changed.
>
> In our test-suite all but 2 tests change the sizes of the files
> involved in the test. That means that all (except these 2) tests don't
> have to depend on the timestamp to complete successfully.
>
> I added (locally) a test to svn_sleep_for_timestamps() to check an
> environment variable whether or not to sleep for stamps. When running
> the test suite with that variable set to the right value,
> basic_tests.py took 24seconds.
>
> Recently lgo added parellel (?) execution of tests. Tests run in
> parellel also take 24seconds.
>
> I'd like to change our test suite to not sleep for timestamps except
> for a specified number of tests. Currently the specified tests would
> be the ones which fail without sleeping. That way, we won't need the
> parellelized feature (lgo prefers to rip that).
>
> Additional benefit of not-sleeping versus parellelizing is that the
> on-disk space-requirements stay the same as they currently are. This
> could be important for execution on RAM disks.
>
>
> Comments?
>
>
> bye,
>
> Erik
>
> The patch to libsvn_subr:svn_sleep_for_timestamps():
>
> Index: subversion/libsvn_subr/time.c
> ===================================================================
> --- subversion/libsvn_subr/time.c (revision 23768)
> +++ subversion/libsvn_subr/time.c (working copy)
> @@ -2,7 +2,7 @@
> * time.c: time/date utilities
> *
> * ====================================================================
> - * Copyright (c) 2000-2004 CollabNet. All rights reserved.
> + * Copyright (c) 2000-2007 CollabNet. All rights reserved.
> *
> * This software is licensed as described in the file COPYING, which
> * you should have received as part of this distribution. The terms
> @@ -289,9 +289,13 @@
> svn_sleep_for_timestamps(void)
> {
> apr_time_t now, then;
> + const char *env_val = getenv("SVN_SLEEP_FOR_TIMESTAMPS");
>
> /* Sleep until the next second tick, plus a tenth of a second for margin. */
> - now = apr_time_now();
> - then = apr_time_make(apr_time_sec(now) + 1, APR_USEC_PER_SEC / 10);
> - apr_sleep(then - now);
> + if (apr_strnatcasecmp(env_val, "no") != 0)
> + {
> + now = apr_time_now();
> + then = apr_time_make(apr_time_sec(now) + 1, APR_USEC_PER_SEC / 10);
> + apr_sleep(then - now);
> + }
> }
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
>
--
Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [RFC] Stop sleeping for timestamps when running tests
Posted by Mattias Engdegård <ma...@virtutech.se>.
Greg Hudson <gh...@MIT.EDU> writes:
>> Can't the client determine that the size has been changed and deduce
>> that it does not need to sleep?
>No; the sleep is there to be able to detect a modification (typically by
>a non-svn command) which happens after the svn operation completes. svn
>cannot know whether that future operation will change the file size or
>not.
Of course. I don't know what I was thinking; sorry about that.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [RFC] Stop sleeping for timestamps when running tests
Posted by Greg Hudson <gh...@MIT.EDU>.
On Mon, 2007-03-12 at 12:32 +0000, Mattias Engdegård wrote:
> Can't the client determine that the size has been changed and deduce
> that it does not need to sleep?
No; the sleep is there to be able to detect a modification (typically by
a non-svn command) which happens after the svn operation completes. svn
cannot know whether that future operation will change the file size or
not.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [RFC] Stop sleeping for timestamps when running tests
Posted by Mattias Engdegård <ma...@virtutech.se>.
"Erik Huelsmann" <eh...@gmail.com> writes:
>I added (locally) a test to svn_sleep_for_timestamps() to check an
>environment variable whether or not to sleep for stamps. When running
>the test suite with that variable set to the right value,
>basic_tests.py took 24seconds.
Can't the client determine that the size has been changed and deduce
that it does not need to sleep? Since it is a fairly common case, it
would improve performance generally instead just being a test hack.
It would also be nice if the client could take advantage of
high-resolution timestamps when these are available. I suppose the
hard part is determining whether the underlying file system supports
it, even when the interface is present.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org