You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Brandon Ehle <az...@yahoo.com> on 2002/11/26 16:18:46 UTC
Entries Cache Performance
Hrmm, actually I just noticed that when I counted my directories
yesterday, I was including the .svn
stuff in the count,what I am really seeing is this:
Working Copy 1
=========
There are 4164 files in the working copy
There are 275 directories in the working copy
There are 271 .svn/entries files in the working copy
XML_Parse() is called 679 times
Of this XML_Parse() time, 42% is spent in svn_time_from_cstring(), and
21% is spent in svn_wc__do_update_cleanup() which is calling
write_entry() (19%).
Working Copy 2
=========
There are 1278 files in the working copy
There are 128 directories in the working copy
There are 107 .svn/entries files in the working copy
XML_Parse() is called 219 times
82% of the time is spent in XML_Parse() and below
Of this XML_Parse() time, 43% is spent in svn_time_from_cstring(), and
22% is spent in svn_wc__do_update_cleanup() which is calling
write_entry() (20%).
Which indicates the XML_Parse is getting run on average of twice per
entries file, plus a couple other calls of XML_Parse() from neon. Any
idea why the entries caching is not working for this case?
Also it appears that expat isn't the source of XML_Parse() being slow,
but rather Subversion's usage of it. Looks like svn_time_from_cstring()
is chilling in sscanf for 99% of its time. On the overall, right now
sscanf is about 33% of svn's time spent when doing a update on an empty
working copy. Any ideas on how to speed this up? Two things that come
to mind is a hand coded replacement for sscanf, or storing the time in
an alternate format where we don't need to use sscanf to parse it.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Entries Cache Performance & fast svn_time_from_cstring code
Posted by Zack Weinberg <za...@codesourcery.com>.
Brandon Ehle <az...@yahoo.com> writes:
> Like I said, it completely fell off the chart when I stopped using
> sscanf, and using fast_strtoul10 didn't really save anything over
> strtoul as far as I can tell, but someone mentioned strtoul it might
> not be very available on every platform, so I rewrote it to be a
> little quicker.
strtoul is in C89; I don't think that's worth worrying about.
zw
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Entries Cache Performance & fast svn_time_from_cstring code
Posted by Brandon Ehle <az...@yahoo.com>.
>
>
>I bet I can squeeze even more cycles out of this. Have fast_strtoul10
>store its result into a pointer argument, since that's going to be a
>memory write anyway, and return the updated pointer. That'll
>facilitate the pointer staying in a register all the time. Also, you
>make heavy use of flag variables in the fast path -- better to handle
>that with jumps out of the fast path.
>
>Note that the 'register' keyword is ignored by most modern compilers,
>so I feel it's best to leave it out.
>
>
Like I said, it completely fell off the chart when I stopped using
sscanf, and using fast_strtoul10 didn't really save anything over
strtoul as far as I can tell, but someone mentioned strtoul it might not
be very available on every platform, so I rewrote it to be a little quicker.
>Is significant time spent in apr_psprintf or children thereof? Both
>scanf and printf -alikes tend to be slow, and I wouldn't be surprised
>if a similar change helped there too.
>
>
Yes, apr_vformatter is the big guy on the printf side, but it isn't as
much of an issue as sscanf.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Entries Cache Performance & fast svn_time_from_cstring code
Posted by Zack Weinberg <za...@codesourcery.com>.
Brandon Ehle <az...@yahoo.com> writes:
> I get about a 14% boost with your patch and additional 5% boost with
> using this version of svn_time_from_cstring(). Its not pretty, but
> its pretty fast.
I bet I can squeeze even more cycles out of this. Have fast_strtoul10
store its result into a pointer argument, since that's going to be a
memory write anyway, and return the updated pointer. That'll
facilitate the pointer staying in a register all the time. Also, you
make heavy use of flag variables in the fast path -- better to handle
that with jumps out of the fast path.
Note that the 'register' keyword is ignored by most modern compilers,
so I feel it's best to leave it out.
> With these two changes svn_time_from_cstring() falls completely off
> the profiler and the next bigger hitter is write_entry().
So perhaps further tuning is not helpful, but I'd be curious how the
appended revision stacks up, anyway.
Is significant time spent in apr_psprintf or children thereof? Both
scanf and printf -alikes tend to be slow, and I wouldn't be surprised
if a similar change helped there too.
zw
=============================================
static const char *
fast_strtoul10(const char *s, unsigned long *result)
{
int c;
unsigned long val = 0;
for (;;) {
c = *s - '0';
if (c < 0 || c > 9)
break;
val = val * 10 + c;
s++;
}
*result = val;
return s;
}
svn_error_t *
svn_time_from_cstring(apr_time_t *when, const char *data, apr_pool_t *pool)
{
apr_time_exp_t exploded_time;
apr_status_t apr_err;
char wday[4], month[4];
const char *c = data;
/* Open-code parsing of the new timestamp format, as this
is a hot path for reading the entries file. */
c = fast_strtoul10(c, &exploded_time.tm_year);
if (*c++ != '-') goto fail;
c = fast_strtoul10(c, &exploded_time.tm_mon);
if (*c++ != '-') goto fail;
c = fast_strtoul10(c, &exploded_time.tm_mday);
if (*c++ != 'T') goto fail;
c = fast_strtoul10(c, &exploded_time.tm_hour);
if (*c++ != ':') goto fail;
c = fast_strtoul10(c, &exploded_time.tm_min);
if (*c++ != ':') goto fail;
c = fast_strtoul10(c, &exploded_time.tm_sec);
if (*c++ != '.') goto fail;
c = fast_strtoul10(c, &exploded_time.tm_usec);
if (*c++ != 'Z') goto fail;
exploded_time.tm_year -= 1900;
exploded_time.tm_mon -= 1;
exploded_time.tm_wday = 0;
exploded_time.tm_yday = 0;
exploded_time.tm_isdst = 0;
exploded_time.tm_gmtoff = 0;
apr_err = apr_implode_gmt(when, &exploded_time);
if (apr_err == APR_SUCCESS)
return SVN_NO_ERROR;
return svn_error_createf(SVN_ERR_BAD_DATE, apr_err, NULL,
"Date conversion failed.");
fail:
/* Try the compatibility option. This does not need to be fast,
as this format is no longer generated and the client will convert
an old-format entries file the first time it reads it. */
if (sscanf(data, old_timestamp_format,
wday,
&exploded_time.tm_mday,
month,
&exploded_time.tm_year,
&exploded_time.tm_hour,
&exploded_time.tm_min,
&exploded_time.tm_sec,
&exploded_time.tm_usec,
&exploded_time.tm_yday,
&exploded_time.tm_isdst,
&exploded_time.tm_gmtoff) == 11)
{
exploded_time.tm_year -= 1900;
exploded_time.tm_yday -= 1;
/* Using hard coded limits for the arrays - they are going away
soon in any case. */
exploded_time.tm_wday = find_matching_string(wday, 7, apr_day_snames);
exploded_time.tm_mon = find_matching_string(month, 12, apr_month_snames);
apr_err = apr_implode_gmt(when, &exploded_time);
if (apr_error == APR_SUCCESS)
return SVN_NO_ERROR;
return svn_error_createf(SVN_ERR_BAD_DATE, apr_err, NULL,
"Date conversion failed.");
}
/* Timestamp is something we do not recognize. */
return svn_error_createf(SVN_ERR_BAD_DATE, 0, NULL,
"Date parsing failed.");
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Entries Cache Performance & fast svn_time_from_cstring code
Posted by Brandon Ehle <az...@yahoo.com>.
>
>
>>>You may like to try the following patch. It should parse the file
>>>only once when there is a write lock.
>>>
>>>
>>I'm seeing about a 14% speed boost with this patch.
>>
>>
>
>Is that elapsed time or CPU or both? Which operation?
>
>I don't see as much as that, I get about 4-5% CPU savings for an
>update which changes 500 out of 1000 files spread over 340
>directories.
>
>
This is elapsed time minus startup time (before the app gets to main).
I get about a 14% boost with your patch and additional 5% boost with
using this version of svn_time_from_cstring(). Its not pretty, but its
pretty fast. With these two changes svn_time_from_cstring() falls
completely off the profiler and the next bigger hitter is write_entry().
=============================================
unsigned long fast_strtoul10(const char *nptr, char **endptr)
{
register const char *s=nptr;
register int c;
register int found;
register unsigned long val=0;
if(!nptr) {
*endptr=(char*)nptr;
return 0;
}
while(1) {
c=*s++;
c -= '0';
if (c<0 || c>9)
break;
found = 1;
val *= 10;
val += c;
}
if(endptr) *endptr=(char*)(found?s-1:nptr);
return val;
}
svn_error_t *
svn_time_from_cstring(apr_time_t *when, const char *data, apr_pool_t *pool)
{
apr_time_exp_t exploded_time;
apr_status_t apr_err;
char wday[4], month[4];
int failed=0;
char *c=NULL;
exploded_time.tm_year=fast_strtoul10(data, &c);
failed+=*c!='-';
exploded_time.tm_mon=fast_strtoul10(c+1, &c);
failed+=*c!='-';
exploded_time.tm_mday=fast_strtoul10(c+1, &c);
failed+=*c!='T';
exploded_time.tm_hour=fast_strtoul10(c+1, &c);
failed+=*c!=':';
exploded_time.tm_min=fast_strtoul10(c+1, &c);
failed+=*c!=':';
exploded_time.tm_sec=fast_strtoul10(c+1, &c);
failed+=*c!='.';
exploded_time.tm_usec=fast_strtoul10(c+1, &c);
failed+=*c!='Z';
// printf("%s -> %d %d %d %d %d %d %d\n", data, exploded_time.tm_year,
exploded_time.tm_mon, exploded_time.tm_mday, exploded_time.tm_hour,
exploded_time.tm_min, exploded_time.tm_sec, exploded_time.tm_usec);
/* First try the new timestamp format. */
if (!failed)
{
exploded_time.tm_year -= 1900;
exploded_time.tm_mon -= 1;
exploded_time.tm_wday = 0;
exploded_time.tm_yday = 0;
exploded_time.tm_isdst = 0;
exploded_time.tm_gmtoff = 0;
apr_err = apr_implode_gmt (when, &exploded_time);
if(apr_err != APR_SUCCESS)
{
return svn_error_createf (SVN_ERR_BAD_DATE, apr_err, NULL,
"Date conversion failed.");
}
return SVN_NO_ERROR;
}
/* Then try the compatibility option. */
else if (sscanf (data,
old_timestamp_format,
wday,
&exploded_time.tm_mday,
month,
&exploded_time.tm_year,
&exploded_time.tm_hour,
&exploded_time.tm_min,
&exploded_time.tm_sec,
&exploded_time.tm_usec,
&exploded_time.tm_yday,
&exploded_time.tm_isdst,
&exploded_time.tm_gmtoff) == 11)
{
exploded_time.tm_year -= 1900;
exploded_time.tm_yday -= 1;
/* Using hard coded limits for the arrays - they are going away
soon in any case. */
exploded_time.tm_wday = find_matching_string (wday, 7,
apr_day_snames);
exploded_time.tm_mon = find_matching_string (month, 12,
apr_month_snames);
apr_err = apr_implode_gmt (when, &exploded_time);
if(apr_err != APR_SUCCESS)
{
return svn_error_createf (SVN_ERR_BAD_DATE, apr_err, NULL,
"Date conversion failed.");
}
return SVN_NO_ERROR;
}
/* Timestamp is something we do not recognize. */
else
{
return svn_error_createf(SVN_ERR_BAD_DATE, 0, NULL,
"Date parsing failed.");
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn --version should give: 0.15.0 (build )
Posted by cm...@collab.net.
Blair Zajac <bl...@orcaware.com> writes:
> I'm +1 on the revision in the build number.
>
> A couple of thoughts to what you said.
>
> One, I think most people that install and use Subversion in a
> production environment would use single revision source trees.
Most people that use Subversion in a production environment will build
from a tarball, which *does* have a revision number instead of "(dev
build)".
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn --version should give: 0.15.0 (build )
Posted by Blair Zajac <bl...@orcaware.com>.
cmpilato@collab.net wrote:
>
> solo turn <so...@yahoo.com> writes:
>
> > could you please add the revision to the output of the --version
> > command?
>
> Not really. I often have a mixed working copy with countless
> different revisions of different files. There is no one version
> number that accurately describes the contents of my compilation.
I'm +1 on the revision in the build number.
A couple of thoughts to what you said.
One, I think most people that install and use Subversion in a
production environment would use single revision source trees.
I'm guessing that mostly its the Subversion development team that
uses revision copies. And then even, do mixed revisions get
installed for use? I know I don't install them, but just test
them. I make my changes, run make check and when I'm happy with
it, I check it in and then build the production svn off of that.
We can make the build process smart enough to look through the
source tree for the revisions of the source files and place them
in the build number. So you could get "build 0.15.0 (revs 3643,
3656, 3800)". If there is more than one revision, then it's clearly
a mixed rev tree and it'll take a closer look to see which files are
at the different revisions. If the rev is a single number, then
its easy to see where it came from.
So for most people, I think having a single number would be good.
For those people who run mixed revisions, they already know they
have a mixed rev.
Finally, I think there's a lot more users of svn then developers
(or we hope there will be in the future) and single rev trees
will be the norm.
Blair
--
Blair Zajac <bl...@orcaware.com>
Plots of your system's performance plots - http://www.orcaware.com/orca/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn --version should give: 0.15.0 (build )
Posted by Daniel Rall <dl...@finemaltcoding.com>.
cmpilato@collab.net writes:
> solo turn <so...@yahoo.com> writes:
>
> > could you please add the revision to the output of the --version
> > command?
>
> Not really. I often have a mixed working copy with countless
> different revisions of different files. There is no one version
> number that accurately describes the contents of my compilation.
At this point it's totally blue sky, but the build could check whether
it has a definitive revision (unmixed/unmodified), and use the
revision number in that case.
--
Daniel Rall <dl...@finemaltcoding.com>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn --version should give: 0.15.0 (build )
Posted by cm...@collab.net.
solo turn <so...@yahoo.com> writes:
> could you please add the revision to the output of the --version
> command?
Not really. I often have a mixed working copy with countless
different revisions of different files. There is no one version
number that accurately describes the contents of my compilation.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn --version should give: 0.15.0 (build )
Posted by Daniel Rall <dl...@finemaltcoding.com>.
solo turn <so...@yahoo.com> writes:
> could you please add the revision to the output of the --version
> command?
>
> "dev build" is NOT clear at all, and can mean so many things. it just
> makes tracking hard, and bugreporting even harder, and finding which
> patch is included in a compiled version impossible.
>
> svn --version should give: 0.15.0 (build <revision>)
I also have often found myself wondering just which revision of the
svn commandline binary I'm running. It would be nice if the server
included this information as well:
Server: Apache/2.0.44-dev (Unix) DAV/2 SVN/0.15.0 (dev build)
--
Daniel Rall <dl...@finemaltcoding.com>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
svn --version should give: 0.15.0 (build )
Posted by solo turn <so...@yahoo.com>.
could you please add the revision to the output of the --version
command?
"dev build" is NOT clear at all, and can mean so many things. it just
makes tracking hard, and bugreporting even harder, and finding which
patch is included in a compiled version impossible.
svn --version should give: 0.15.0 (build <revision>)
...
__________________________________________________
Do you Yahoo!?
Yahoo! Mail Plus - Powerful. Affordable. Sign up now.
http://mailplus.yahoo.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Entries Cache Performance
Posted by Philip Martin <ph...@codematters.co.uk>.
Brandon Ehle <az...@yahoo.com> writes:
> > You may like to try the following patch. It should parse the file
> > only once when there is a write lock.
>
> I'm seeing about a 14% speed boost with this patch.
Is that elapsed time or CPU or both? Which operation?
I don't see as much as that, I get about 4-5% CPU savings for an
update which changes 500 out of 1000 files spread over 340
directories.
>
> > /* We ask for the deleted entries if there is a write lock on the
> > basis that we will eventually need these when we come to write.
> > Getting them now avoids a second file parse. However if we don't
> > ever write it does use more memory. */
>
> But something odd is happening, I am seeing a memory *savings* after applying this patch, unless of course I am reading /proc wrong?
Interesting, I had not considered that. I did put in an optimisation,
some time ago, that makes the without-deleted entries set use the same
memory as the with-deleted set if there are no deleted entries and the
with-deleted set already exists. It's likely that my patch increases
the chances of this occurring. It's also likely you don't have any
deleted entries (items in the deleted state).
Overall, I think a patch along these lines is an improvement.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Entries Cache Performance
Posted by Brandon Ehle <az...@yahoo.com>.
Greg Hudson wrote:
>On Tue, 2002-11-26 at 11:18, Brandon Ehle wrote:
>
>
>>On the overall, right now
>>sscanf is about 33% of svn's time spent when doing a update on an empty
>>working copy. Any ideas on how to speed this up? Two things that come
>>to mind is a hand coded replacement for sscanf, or storing the time in
>>an alternate format where we don't need to use sscanf to parse it.
>>
>>
>
>Yeah, those are the two basic strategies. First, switch to hand-parsing
>the date, which shouldn't be too difficult; if that doesn't cut down the
>time spent in svn_time_from_cstring sufficiently, switch to storing
>dates in the entries file as decimal apr_time_t values. (Which is less
>readable, but simple and fast.)
>
>
Ok, I am going to try and implement a faster svn_time_from_cstring today
and see what kind of results I get.
>> Also it appears that expat isn't the source of XML_Parse() being slow,
>> but rather Subversion's usage of it. Looks like
>> svn_time_from_cstring() is chilling in sscanf for 99% of its time. On
>> the overall, right now sscanf is about 33% of svn's time spent when
>> doing a update on an empty working copy. Any ideas on how to speed
>> this up? Two things that come to mind is a hand coded replacement for
>> sscanf, or storing the time in an alternate format where we don't need
>> to use sscanf to parse it.
> Which platform?
This was run on RedHat 8.0.
> You may like to try the following patch. It should parse the file
> only once when there is a write lock.
I'm seeing about a 14% speed boost with this patch.
> /* We ask for the deleted entries if there is a write lock on the
> basis that we will eventually need these when we come to write.
> Getting them now avoids a second file parse. However if we don't
> ever write it does use more memory. */
But something odd is happening, I am seeing a memory *savings* after applying this patch, unless of course I am reading /proc wrong?
Before Patch (avg time is 1.85 seconds)
=========
VmSize: 15048 kB
VmLck: 0 kB
VmRSS: 9756 kB
VmData: 8012 kB
VmStk: 144 kB
VmExe: 64 kB
VmLib: 4328 kB
After Patch (avg time is 1.62 seconds)
=========
VmSize: 11736 kB
VmLck: 0 kB
VmRSS: 6560 kB
VmData: 4700 kB
VmStk: 144 kB
VmExe: 64 kB
VmLib: 4328 kB
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Entries Cache Performance
Posted by Greg Hudson <gh...@MIT.EDU>.
On Tue, 2002-11-26 at 11:18, Brandon Ehle wrote:
> On the overall, right now
> sscanf is about 33% of svn's time spent when doing a update on an empty
> working copy. Any ideas on how to speed this up? Two things that come
> to mind is a hand coded replacement for sscanf, or storing the time in
> an alternate format where we don't need to use sscanf to parse it.
Yeah, those are the two basic strategies. First, switch to hand-parsing
the date, which shouldn't be too difficult; if that doesn't cut down the
time spent in svn_time_from_cstring sufficiently, switch to storing
dates in the entries file as decimal apr_time_t values. (Which is less
readable, but simple and fast.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Entries Cache Performance
Posted by Philip Martin <ph...@codematters.co.uk>.
Brandon Ehle <az...@yahoo.com> writes:
> Which indicates the XML_Parse is getting run on average of twice per
> entries file, plus a couple other calls of XML_Parse() from neon. Any
> idea why the entries caching is not working for this case?
Entries caching is working, before caching the XML_Parse would have
been called lots of times for each file. As I explained earlier,
there are two sets of entries and the order in which the sets are
requested determines whether the file needs to be read twice.
You may like to try the following patch. It should parse the file
only once when there is a write lock.
Index: subversion/libsvn_wc/lock.c
===================================================================
--- subversion/libsvn_wc/lock.c (revision 3902)
+++ subversion/libsvn_wc/lock.c (working copy)
@@ -299,7 +299,11 @@
apr_hash_index_t *hi;
apr_pool_t *subpool = svn_pool_create (pool);
- SVN_ERR (svn_wc_entries_read (&entries, lock, FALSE, subpool));
+ /* We ask for the deleted entries if there is a write lock on the
+ basis that we will eventually need these when we come to write.
+ Getting them now avoids a second file parse. However if we don't
+ ever write it does use more memory. */
+ SVN_ERR (svn_wc_entries_read (&entries, lock, write_lock, subpool));
/* Use a temporary hash until all children have been opened. */
if (associated)
@@ -317,7 +321,7 @@
apr_hash_this (hi, NULL, NULL, &val);
entry = val;
- if (entry->kind != svn_node_dir
+ if (entry->deleted || entry->kind != svn_node_dir
|| ! strcmp (entry->name, SVN_WC_ENTRY_THIS_DIR))
continue;
entry_path = svn_path_join (lock->path, entry->name, subpool);
> Also it appears that expat isn't the source of XML_Parse() being slow,
> but rather Subversion's usage of it. Looks like
> svn_time_from_cstring() is chilling in sscanf for 99% of its time. On
> the overall, right now sscanf is about 33% of svn's time spent when
> doing a update on an empty working copy. Any ideas on how to speed
> this up? Two things that come to mind is a hand coded replacement for
> sscanf, or storing the time in an alternate format where we don't need
> to use sscanf to parse it.
Which platform?
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org