On Thu, May 15, 2014 at 2:43 PM, Bert Huijben <bert_at_qqmail.nl> wrote:
>
>
> > -----Original Message-----
> > From: stefan2_at_apache.org [mailto:stefan2_at_apache.org]
> > Sent: woensdag 7 mei 2014 15:44
> > To: commits_at_subversion.apache.org
> > Subject: svn commit: r1593015 - in
> > /subversion/trunk/subversion/libsvn_fs_fs: fs.c fs.h
> >
> > Author: stefan2
> > Date: Wed May 7 13:43:55 2014
> > New Revision: 1593015
> >
> > URL: http://svn.apache.org/r1593015
> > Log:
> > Follow-up to r1591919: Fix fs tests on Windows and non-threaded platforms
> > by always using our svn_mutex__t and thus being able to detect recursive
> > locking attempts.
> >
> > For non-threaded platforms, always using svn_mutex__t adds almost zero
> > overhead to the FS file locking that we need to do anyways. On Windows,
> > the overhead slightly larger but all the APR functions that svn_mutex__*
> > calls have very efficient implementations on Windows. So here, the
> relative
> > overhead is minimal as well.
> >
> > OTOH, all our tested platforms will detect incorrect / non-portable lock
> > usage.
>
> Not that even after a week the OpenBSD tests are still failing.
>
Sorry about that! I've been travelling, had checked only for
the FSFS bits to test ok on the build bot and did not recheck
later. r1595573 seems to finally fix it (we'll know in about 5h)
> And I'm not sure if this is really the right way to fix such a corner case
> on Windows.
>
> Why should we do extra work on Windows which has obvious performance side
> effects?
>
The performance impact on Windows should be less 200ns per
lock assuming that APR has been built with intrinsics enabled.
Even with 3 times that amount, it means that a full (dump+) load
of the ASF repo gets about 10s slower - while taking > 10h total.
Windows does check per handle, which is really what we need here. We add
> the missing checks for platforms that don't check, but adding a check on
> platforms that don't need it really make sense.
>
So, attempts to take out a lock twice (within the same thread)
does not result in deadlocks? Our internal API sure makes it
a possibility and the *public* svn_fs_freeze can be used recursively
as well. Exposing this function in our bindings makes things
worse when they try to debug the problem from their respective
environments.
I understand that the issues caused by two repos falsely sharing
the same FS data instance do not exist under WIndows as the
repo lock files are still disjoint. IOW, you are right that the deadlock
case is more edgy under Windows than on other platforms.
I'll remind you of this example the next time you call the Windows
> implementation slow.
Our Windows code is slow for many reasons but critical sections
are none of them. In our case, they are not contented and take
about 10ns to acquire. The about 200ns figure is grossly bolstered
up to account for APR internal and calling overhead as well as
releasing the lock.
Sure, Windows is slow if you first want to make it behave exactly like *nix
> and then add another layer for the features missing on *nix.
>
I want our code to be (at least) as robust under Windows as on
other platforms. If you are willing to trade a << 0.1% performance
again against a theoretical deadlock, then say so. Or demonstrate
that either my estimates or the technical rationale behind the
deadlock check are wrong.
> Same thing as we used to do for obtaining file status... Read the
> directory; and then for every node read the directory again because we
> assume every system uses i-nodes to hold other things.
>
Without looking at the code, that sounds like a fair case for
platform specific code. Most of our perceived performance
issues on Windows are in the working copy code. All power
to you if you feel that platform specific code will work better.
> These 'performance optimizations' you have been applying over the last few
> weeks are slowed Subversion down by +- 2-5% on my test environment when I
> measured the result about a week ago.
>
I remember seeing you comment something like this on IRC
(when I scanned the log) and I took it as a "it's seems slower
but I can't see why" statement. That weekend I only committed
the 'svn log' error code issue and the stream chunk size increase.
Neither of which are immediately plausible as a root cause.
Can you identify which of the commits in the [1586922:1586953]
range causes the slowdown you are seeing?
My numbers for ra_local+fsfs over a wider range of revisions:
r1577835 (2049 tests) real 87.656s (STD=0.832s), user+sys 495.611s
(STD=4.197s)
r1583644 (2053 tests) real 87.554s (STD=1.368s), user+sys 495.191s
(STD=8.312s)
r1593146 (2064 tests) real 88.444s (STD=0.692s), user+sys 499.128s
(STD=4.468s)
Averages and standard deviation of 3 runs of our test suite with
'time make check PARALLEL=16 > /dev/null' on a ramdisk on
my macbook with a static, fully optimized build. Normalized to
the same number of tests, the results are almost perfectly stable.
> Whatever runs faster on the combination of your huge system, your specific
> compiler, your os and your multi gbit network is not necessarily whatever
> all others are using.
>
True. But testing with a macbook against a HDD based server
with RAM < repo size over a 1Gbit network seems like a fair
test to me. And *then* testing to server throughput and scalability
by simulating a network of clients using stock hardware is certainly
reasonable as well.
OTOH, using the test suite as a benchmark will yield indirect
results at best: once you verified that there is a slowdown and
identified the root cause, you still have to evaluate the impact
on people that don't work on greek trees all day.
> And optimizing for most of those specific scenarios is a task for the OS,
> compiler, platform libraries etc. Not necessarily always Subversion itself.
If the application allocates >30GB of dynamic memory as it was
the case until a few weeks ago, $MagicToolOrComponent will
have a hard time preventing a crash.
As for the 'svn log', we got bitten by limiting our allocator to 4MB
of unused memory while Apache's default is 64MB (?) or so. Hence,
svnserve was less than half as fast as httpd. Moreover, we would
allocate several 100MB per client during "svn log -g". Both issues
have been addressed in Subversion itself now.
> (E.g. in the recent cases where code was added to support more memory for
> apr compiled in a nonstandard configuration)
>
Using mmap for allocations *is* the standard configuration on
some systems. Simply aborting with OOM without indicating
what the actual root cause might be is just bad style.
-- Stefan^2.
Received on 2014-05-19 08:36:36 CEST