On Thu, Sep 18, 2014 at 3:00 PM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
> On 11 September 2014 20:28, Stefan Fuhrmann
> <stefan.fuhrmann_at_wandisco.com> wrote:
> > On Wed, Sep 10, 2014 at 5:35 PM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
> >>
> >> I've accidentally ran Subversion test suite under elevated local
> >> administrator account on Windows and got failures in named-atomics
> >> tests:
> >> [[[
> >> ..\..\..\subversion\tests\libsvn_subr\named_atomic-test.c:520:
> >> (apr_err=SVN_ERR_TEST_FAILED)
> >> svn_tests: E200006: assertion 'value == 42' failed at
> >> ..\..\..\subversion\tests\libsvn_subr\named_atomic-test.c:520
> >> FAIL: named_atomic-test.exe 1: basic r/w access to a single atomic
> >> ..\..\..\subversion\tests\libsvn_subr\named_atomic-test.c:581:
> >> (apr_err=SVN_ERR_TEST_FAILED)
> >> svn_tests: E200006: assertion 'value == 42 * HUGE_VALUE' failed at
> >> ..\..\..\subversion\tests\libsvn_subr\named_atomic-test.c:581
> >> FAIL: named_atomic-test.exe 2: atomics must be 64 bits
> >> ..\..\..\subversion\tests\libsvn_subr\named_atomic-test.c:643:
> >> (apr_err=SVN_ERR_TEST_FAILED)
> >> svn_tests: E200006: assertion 'value1 == 46 * HUGE_VALUE' failed at
> >> ..\..\..\subversion\tests\libsvn_subr\named_atomic-test.c:643
> >> FAIL: named_atomic-test.exe 3: basic r/w access to multiple atomics
> >> ]]]
> >>
> >> The investigation revealed the following:
> >> 1. The named_atomics tests are skipped unless executed under elevated
> >> local administrator on Windows.
> >
> >
> > What worries me more is that our Windows build bots don't report
> > those tests as "SKIP" - although the common init_test_shm() function
> > in named_atomics-test.c will return SVN_ERR_TEST_SKIPPED.
> > Are they reported as skipped in your setup when you run the test
> > without local admin rights?
> >
> I'm getting SKIP notifications for named_atomics tests when run them
> without local admin rights. It seems that buildbot executes these tests,
> but Windows buildbot uses VS2010 which doesn't have intrinsics for
> "efficient" atomics, so it uses different code.
>
Yes, I identified 32 bits with old SDKs as the ones that
won't use intrinsics and, therefore, wouldn't use revprop
caching at all.
[...]
>> 2. Remove it since "named atomics" framework is only used for currently
> >> disabled revprop caching.
> >
> >
> > ... I'm +1 on getting rid of the SHM code altogether (we are using MMAP
> to
> > get shared memory). It turned out to be a poor choice as all sorts of
> > Windows
> > platform specifics make it hard caused us to deviate further and further
> > from
> > the initial APR-based design. Some of the Windows-specific issues, e.g.
> the
> > file retry races in the init code, should have been reported before the
> > 1.8.0
> > release.
> >
> >>
> >> Personally I don't see reason to spend resources fixing unused code,
> >> especially
> >> that even code on 'revprop-caching-ng' branch removes it also. Any
> >> other opinions?
> >
> >
> > No, I agree.
> >
> > The revpro-caching-ng branch pretty much exactly removes the SHM
> > code.
> I think that the revprop-caching-ng branch should not be merged. So
> may be worth to remove named-atomics and SHM code from trunk to turn
> the trunk back to the normal state (without unused and not working code)
>
Here is what I will do on /trunk:
* Merge the new revprop caching scheme to FSX.
* Remove the SHM-dependent bits from FSFS but keep the actual
cache invocations (no-ops since there is no cache instance) at least
for now.
* Remove the named_atomics code.
* Update the revprop-caching-ng branch
Why do you think the revprop-caching-ng branch should not be merged?
-- Stefan^2.
Received on 2014-09-23 12:32:20 CEST