You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Fuhrmann <st...@alice-dsl.de> on 2010/08/18 19:14:48 UTC
Performance branch ready for review
Hi @all,
I just finished my porting work; the performance branch
is now fully synchronized with my prototype code.
From my point of view, review can start now.
According to my measurements, the code is now faster
than the original prototype. Large caches provided, a
single multi-threaded svnserve instance on a modern
quad-core machine should be able to saturate a 10Gb
server connection.
Open issues / things still to do
* there is an issue with "log" triggering an assertion()
-> I will investigate that next
* test mod_web_dav and add FSFS cache configuration
parameters to it
* tune membuffer cache eviction strategy such that even
small caches can have a large impact
* add tests for the new APIs
* provide APR patches.
There are many things I would like to do but they may
better be deferred to 1.8.
-- Stefan^2.
Re: Performance branch ready for review
Posted by Johan Corveleyn <jc...@gmail.com>.
On Tue, Aug 24, 2010 at 12:22 AM, Stefan Fuhrmann
<st...@alice-dsl.de> wrote:
> Daniel Shahaf wrote:
>>
>> Stefan, you did mention "Patch by" for Johan's patches which you
>> committed, did you intend to mention "Found by" or "Suggested by" for
>> the other two (quoted below)?
>>
>>
>> http://subversion.apache.org/docs/community-guide/conventions.html#crediting
>>
>> Thanks,
>>
>> Daniel
>>
>
> Oh, I just was not aware that there are tons of "... by" schemes.
> r987868 and r987869 now rightfully mention Johan.
Thanks. Not terribly important to me, but nice anyway.
I just hope some of your performance-work makes it into 1.7. So
anything I can do to help ...
Cheers,
--
Johan
Re: Performance branch ready for review
Posted by Stefan Fuhrmann <st...@alice-dsl.de>.
Daniel Shahaf wrote:
> Stefan, you did mention "Patch by" for Johan's patches which you
> committed, did you intend to mention "Found by" or "Suggested by" for
> the other two (quoted below)?
>
> http://subversion.apache.org/docs/community-guide/conventions.html#crediting
>
> Thanks,
>
> Daniel
>
Oh, I just was not aware that there are tons of "... by" schemes.
r987868 and r987869 now rightfully mention Johan.
-- Stefan^2.
Re: Performance branch ready for review
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan, you did mention "Patch by" for Johan's patches which you
committed, did you intend to mention "Found by" or "Suggested by" for
the other two (quoted below)?
http://subversion.apache.org/docs/community-guide/conventions.html#crediting
Thanks,
Daniel
Stefan Fuhrmann wrote on Sun, Aug 22, 2010 at 14:56:46 +0200:
> Johan Corveleyn wrote:
>> 4) In temp_serializer.c, some of the sizeof statements gave problems for me:
>> I changed them as follows (not sure if this is correct, but this compiles ok):
>> [[[
>> Index: subversion/libsvn_fs_fs/temp_serializer.c
>> ===================================================================
>> --- subversion/libsvn_fs_fs/temp_serializer.c (revision 986928)
>> +++ subversion/libsvn_fs_fs/temp_serializer.c (working copy)
>> @@ -271,7 +271,7 @@
>> ]]]
>>
>>
> I think your patch would work in almost all real-world applications.
> As I understand it, however, [...]
>
> Fixed in r987869.
>> I haven't been able to resolve these ...
>>
> Fixed in r987886.
Re: Performance branch ready for review
Posted by Johan Corveleyn <jc...@gmail.com>.
On Sun, Aug 22, 2010 at 2:56 PM, Stefan Fuhrmann
<st...@alice-dsl.de> wrote:
> Johan Corveleyn wrote:
>>
>> On Wed, Aug 18, 2010 at 9:14 PM, Stefan Fuhrmann
>> <st...@alice-dsl.de> wrote:
>>
>>>
>>> Hi @all,
>>>
>>> I just finished my porting work; the performance branch
>>> is now fully synchronized with my prototype code.
>>> From my point of view, review can start now.
>>>
>>> According to my measurements, the code is now faster
>>> than the original prototype. Large caches provided, a
>>> single multi-threaded svnserve instance on a modern
>>> quad-core machine should be able to saturate a 10Gb
>>> server connection.
>>>
>>> Open issues / things still to do
>>>
>>> * there is an issue with "log" triggering an assertion()
>>> -> I will investigate that next
>>> * test mod_web_dav and add FSFS cache configuration
>>> parameters to it
>>> * tune membuffer cache eviction strategy such that even
>>> small caches can have a large impact
>>> * add tests for the new APIs
>>> * provide APR patches.
>>>
>>> There are many things I would like to do but they may
>>> better be deferred to 1.8.
>>>
>>
>> I tried compiling your branch on Windows (XP) with Visual C++ Express
>> 2008 (which I also use successfully to build trunk). I had a couple of
>> issues. FWIW I'm listing them here. I'm not an expert, just
>> pragmatically trying to get the thing to build, so some of these
>> things may be "user error". Eventually, I was able to build svn.exe,
>> but svnadmin.exe and svnserve.exe still fail for me.
>>
>> 1) In build.conf, I added private\svn_temp_serializer.h and
>> private\svn_file_handle_cache.h to the list of msvc-export of
>> libsvn_subr. Otherwise, the linker gave problems:
>>
>> libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
>> symbol _svn_file_handle_cache__has_file referenced in function
>> _svn_fs_fs__path_rev_absolute
>> libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
>> symbol _svn_stream__from_cached_file_handle referenced in function
>> _get_node_revision_body
>> libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
>> symbol _svn_file_handle_cache__open referenced in function
>> _get_node_revision_body
>> libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
>> symbol _svn_file_handle_cache__get_apr_handle referenced in function
>> _open_pack_or_rev_file
>> libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
>> symbol _svn_file_handle_cache__flush referenced in function
>> _sync_file_handle_cache
>> libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
>> symbol _svn_file_handle_cache__close referenced in function
>> _svn_fs_fs__rev_get_root
>> libsvn_fs_fs-1.lib(caching.obj) : error LNK2019: unresolved external
>> symbol _svn_file_handle_cache__create_cache referenced in function
>> _get_global_file_handle_cache
>> libsvn_fs_fs-1.lib(id.obj) : error LNK2019: unresolved external symbol
>> _svn_temp_serializer__pop referenced in function
>> _svn_fs_fs__id_serialize
>> libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2001: unresolved
>> external symbol _svn_temp_serializer__pop
>> libsvn_fs_fs-1.lib(id.obj) : error LNK2019: unresolved external symbol
>> _svn_temp_serializer__push referenced in function
>> _svn_fs_fs__id_serialize
>> libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2001: unresolved
>> external symbol _svn_temp_serializer__push
>> libsvn_fs_fs-1.lib(id.obj) : error LNK2019: unresolved external symbol
>> _svn_temp_serializer__add_string referenced in function
>> _serialize_id_private
>> libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2001: unresolved
>> external symbol _svn_temp_serializer__add_string
>> libsvn_fs_fs-1.lib(dag.obj) : error LNK2001: unresolved external
>> symbol _svn_temp_serializer__add_string
>> libsvn_fs_fs-1.lib(id.obj) : error LNK2019: unresolved external symbol
>> _svn_temp_deserializer__resolve referenced in function
>> _svn_fs_fs__id_deserialize
>> libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2001: unresolved
>> external symbol _svn_temp_deserializer__resolve
>> libsvn_fs_fs-1.lib(dag.obj) : error LNK2001: unresolved external
>> symbol _svn_temp_deserializer__resolve
>> libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2019: unresolved
>> external symbol _svn_temp_serializer__get referenced in function
>> _svn_fs_fs__serialize_txdelta_window
>> libsvn_fs_fs-1.lib(dag.obj) : error LNK2001: unresolved external
>> symbol _svn_temp_serializer__get
>> libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2019: unresolved
>> external symbol _svn_temp_serializer__init referenced in function
>> _svn_fs_fs__serialize_txdelta_window
>> libsvn_fs_fs-1.lib(dag.obj) : error LNK2001: unresolved external
>> symbol _svn_temp_serializer__init
>> libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2019: unresolved
>> external symbol _svn_temp_deserializer__ptr referenced in function
>> _svn_fs_fs__extract_dir_entry
>> libsvn_fs_fs-1.lib(dag.obj) : error LNK2019: unresolved external
>> symbol _svn_temp_serializer__set_null referenced in function
>> _svn_fs_fs__dag_serialize
>> ..\..\..\Debug\subversion\libsvn_fs\libsvn_fs-1.dll : fatal error
>> LNK1120: 15 unresolved externals
>>
>> Here's a patch that solves this for me:
>> [[[
>> Index: build.conf
>> ===================================================================
>> --- build.conf (revision 986928)
>> +++ build.conf (working copy)
>> @@ -322,6 +322,7 @@
>> private\svn_opt_private.h private\svn_skel.h private\svn_sqlite.h
>> private\svn_utf_private.h private\svn_eol_private.h
>> private\svn_token.h
>> + private\svn_temp_serializer.h private\svn_file_handle_cache.h
>>
>> # Working copy management lib
>> [libsvn_wc]
>> ]]]
>>
>
> Committed as r987875.
>>
>> 2) The NULL in svn_temp_serializer__set_NULL in svn_temp_serializer_h
>> also gave the linker a hard time:
>>
>> libsvn_fs_fs-1.lib(dag.obj) : error LNK2019: unresolved external
>> symbol _svn_temp_serializer__set_NULL referenced in function
>> _svn_fs_fs__dag_serialize
>> ..\..\..\Debug\subversion\libsvn_fs\libsvn_fs-1.dll : fatal error
>> LNK1120: 1 unresolved externals
>>
>> Changing the NULL to null (and updating callers) fixed that.
>>
>
> Committed as r987872.
>>
>> 3) In function store_current_end_pointer in svn_temp_serializer_c,
>> target_ptr needs to be declared before the statements.
>>
>>
>> 4) In temp_serializer.c, some of the sizeof statements gave problems for
>> me:
>>
>> ..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(274): error C2057:
>> expected constant expression
>> ..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(274): error C2466:
>> cannot allocate an array of constant size 0
>> ..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(274): warning
>> C4034: sizeof returns 0
>> ..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(424): error C2057:
>> expected constant expression
>> ..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(424): error C2466:
>> cannot allocate an array of constant size 0
>> ..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(424): warning
>> C4034: sizeof returns 0
>>
>> I changed them as follows (not sure if this is correct, but this compiles
>> ok):
>> [[[
>> Index: subversion/libsvn_fs_fs/temp_serializer.c
>> ===================================================================
>> --- subversion/libsvn_fs_fs/temp_serializer.c (revision 986928)
>> +++ subversion/libsvn_fs_fs/temp_serializer.c (working copy)
>> @@ -271,7 +271,7 @@
>>
>> /* calculate sizes */
>> apr_size_t count = apr_hash_count(entries);
>> - apr_size_t entries_len = sizeof(svn_fs_dirent_t*[count]);
>> + apr_size_t entries_len = sizeof(svn_fs_dirent_t*) * count;
>>
>> /* copy the hash entries to an auxilliary struct of known layout */
>> hash_data.count = count;
>> @@ -421,7 +421,7 @@
>> /* the ops form a simple chunk of memory with no further references */
>> svn_temp_serializer__push(context,
>> (const void * const *)ops,
>> - sizeof(svn_txdelta_op_t[count]));
>> + sizeof(svn_txdelta_op_t) * count);
>> svn_temp_serializer__pop(context);
>> }
>>
>> ]]]
>>
>>
>
> I think your patch would work in almost all real-world applications.
> As I understand it, however, sizeof(T) is not the same as sizeof(T[1])
> if T is not of a naturally aligning size. The size of svn_txdelta_op_t, for
> instance is not necessarily a multiple of 16. If the compiler is instructed
> to align data to 16 byte boundaries, the difference becomes important.
>
> Fixed in r987869.
>>
>> 5) Got the following error from mod_dav_svn/repos.c:
>> ..\..\..\subversion\mod_dav_svn\repos.c(1522): error C4013:
>> 'svn_ctype_isspace' undefined; assuming extern returning int
>>
>> Adding an #include "svn_ctype.h" fixed that, but I'm not too sure if
>> that's ok.
>>
>>
>
> Committed as r987893.
>>
>> After these changes, I can get svn.exe to build. For svnadmin and
>> svnserve, I still get linker errors like:
>>
>> main.obj : error LNK2019: unresolved external symbol
>> _svn_fs_fs__set_cache_config referenced in function _main
>> main.obj : error LNK2019: unresolved external symbol
>> _svn_fs_fs__get_cache_config referenced in function _main
>> ..\..\..\Debug\subversion\svnadmin\svnadmin.exe : fatal error LNK1120:
>> 2 unresolved externals
>>
>>
>> I haven't been able to resolve these ...
>>
>
> Fixed in r987886.
>
> Thanks for the feedback!
>
> -- Stefan^2.
>
Ok, cool. Now it all builds correctly. I'll give it a spin :-).
Cheers,
--
Johan
Re: Performance branch ready for review
Posted by Stefan Fuhrmann <st...@alice-dsl.de>.
Johan Corveleyn wrote:
> On Wed, Aug 18, 2010 at 9:14 PM, Stefan Fuhrmann
> <st...@alice-dsl.de> wrote:
>
>> Hi @all,
>>
>> I just finished my porting work; the performance branch
>> is now fully synchronized with my prototype code.
>> From my point of view, review can start now.
>>
>> According to my measurements, the code is now faster
>> than the original prototype. Large caches provided, a
>> single multi-threaded svnserve instance on a modern
>> quad-core machine should be able to saturate a 10Gb
>> server connection.
>>
>> Open issues / things still to do
>>
>> * there is an issue with "log" triggering an assertion()
>> -> I will investigate that next
>> * test mod_web_dav and add FSFS cache configuration
>> parameters to it
>> * tune membuffer cache eviction strategy such that even
>> small caches can have a large impact
>> * add tests for the new APIs
>> * provide APR patches.
>>
>> There are many things I would like to do but they may
>> better be deferred to 1.8.
>>
>
> I tried compiling your branch on Windows (XP) with Visual C++ Express
> 2008 (which I also use successfully to build trunk). I had a couple of
> issues. FWIW I'm listing them here. I'm not an expert, just
> pragmatically trying to get the thing to build, so some of these
> things may be "user error". Eventually, I was able to build svn.exe,
> but svnadmin.exe and svnserve.exe still fail for me.
>
> 1) In build.conf, I added private\svn_temp_serializer.h and
> private\svn_file_handle_cache.h to the list of msvc-export of
> libsvn_subr. Otherwise, the linker gave problems:
>
> libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
> symbol _svn_file_handle_cache__has_file referenced in function
> _svn_fs_fs__path_rev_absolute
> libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
> symbol _svn_stream__from_cached_file_handle referenced in function
> _get_node_revision_body
> libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
> symbol _svn_file_handle_cache__open referenced in function
> _get_node_revision_body
> libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
> symbol _svn_file_handle_cache__get_apr_handle referenced in function
> _open_pack_or_rev_file
> libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
> symbol _svn_file_handle_cache__flush referenced in function
> _sync_file_handle_cache
> libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
> symbol _svn_file_handle_cache__close referenced in function
> _svn_fs_fs__rev_get_root
> libsvn_fs_fs-1.lib(caching.obj) : error LNK2019: unresolved external
> symbol _svn_file_handle_cache__create_cache referenced in function
> _get_global_file_handle_cache
> libsvn_fs_fs-1.lib(id.obj) : error LNK2019: unresolved external symbol
> _svn_temp_serializer__pop referenced in function
> _svn_fs_fs__id_serialize
> libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2001: unresolved
> external symbol _svn_temp_serializer__pop
> libsvn_fs_fs-1.lib(id.obj) : error LNK2019: unresolved external symbol
> _svn_temp_serializer__push referenced in function
> _svn_fs_fs__id_serialize
> libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2001: unresolved
> external symbol _svn_temp_serializer__push
> libsvn_fs_fs-1.lib(id.obj) : error LNK2019: unresolved external symbol
> _svn_temp_serializer__add_string referenced in function
> _serialize_id_private
> libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2001: unresolved
> external symbol _svn_temp_serializer__add_string
> libsvn_fs_fs-1.lib(dag.obj) : error LNK2001: unresolved external
> symbol _svn_temp_serializer__add_string
> libsvn_fs_fs-1.lib(id.obj) : error LNK2019: unresolved external symbol
> _svn_temp_deserializer__resolve referenced in function
> _svn_fs_fs__id_deserialize
> libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2001: unresolved
> external symbol _svn_temp_deserializer__resolve
> libsvn_fs_fs-1.lib(dag.obj) : error LNK2001: unresolved external
> symbol _svn_temp_deserializer__resolve
> libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2019: unresolved
> external symbol _svn_temp_serializer__get referenced in function
> _svn_fs_fs__serialize_txdelta_window
> libsvn_fs_fs-1.lib(dag.obj) : error LNK2001: unresolved external
> symbol _svn_temp_serializer__get
> libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2019: unresolved
> external symbol _svn_temp_serializer__init referenced in function
> _svn_fs_fs__serialize_txdelta_window
> libsvn_fs_fs-1.lib(dag.obj) : error LNK2001: unresolved external
> symbol _svn_temp_serializer__init
> libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2019: unresolved
> external symbol _svn_temp_deserializer__ptr referenced in function
> _svn_fs_fs__extract_dir_entry
> libsvn_fs_fs-1.lib(dag.obj) : error LNK2019: unresolved external
> symbol _svn_temp_serializer__set_null referenced in function
> _svn_fs_fs__dag_serialize
> ..\..\..\Debug\subversion\libsvn_fs\libsvn_fs-1.dll : fatal error
> LNK1120: 15 unresolved externals
>
> Here's a patch that solves this for me:
> [[[
> Index: build.conf
> ===================================================================
> --- build.conf (revision 986928)
> +++ build.conf (working copy)
> @@ -322,6 +322,7 @@
> private\svn_opt_private.h private\svn_skel.h private\svn_sqlite.h
> private\svn_utf_private.h private\svn_eol_private.h
> private\svn_token.h
> + private\svn_temp_serializer.h private\svn_file_handle_cache.h
>
> # Working copy management lib
> [libsvn_wc]
> ]]]
>
Committed as r987875.
> 2) The NULL in svn_temp_serializer__set_NULL in svn_temp_serializer_h
> also gave the linker a hard time:
>
> libsvn_fs_fs-1.lib(dag.obj) : error LNK2019: unresolved external
> symbol _svn_temp_serializer__set_NULL referenced in function
> _svn_fs_fs__dag_serialize
> ..\..\..\Debug\subversion\libsvn_fs\libsvn_fs-1.dll : fatal error
> LNK1120: 1 unresolved externals
>
> Changing the NULL to null (and updating callers) fixed that.
>
Committed as r987872.
>
> 3) In function store_current_end_pointer in svn_temp_serializer_c,
> target_ptr needs to be declared before the statements.
>
>
> 4) In temp_serializer.c, some of the sizeof statements gave problems for me:
>
> ..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(274): error C2057:
> expected constant expression
> ..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(274): error C2466:
> cannot allocate an array of constant size 0
> ..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(274): warning
> C4034: sizeof returns 0
> ..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(424): error C2057:
> expected constant expression
> ..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(424): error C2466:
> cannot allocate an array of constant size 0
> ..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(424): warning
> C4034: sizeof returns 0
>
> I changed them as follows (not sure if this is correct, but this compiles ok):
> [[[
> Index: subversion/libsvn_fs_fs/temp_serializer.c
> ===================================================================
> --- subversion/libsvn_fs_fs/temp_serializer.c (revision 986928)
> +++ subversion/libsvn_fs_fs/temp_serializer.c (working copy)
> @@ -271,7 +271,7 @@
>
> /* calculate sizes */
> apr_size_t count = apr_hash_count(entries);
> - apr_size_t entries_len = sizeof(svn_fs_dirent_t*[count]);
> + apr_size_t entries_len = sizeof(svn_fs_dirent_t*) * count;
>
> /* copy the hash entries to an auxilliary struct of known layout */
> hash_data.count = count;
> @@ -421,7 +421,7 @@
> /* the ops form a simple chunk of memory with no further references */
> svn_temp_serializer__push(context,
> (const void * const *)ops,
> - sizeof(svn_txdelta_op_t[count]));
> + sizeof(svn_txdelta_op_t) * count);
> svn_temp_serializer__pop(context);
> }
>
> ]]]
>
>
I think your patch would work in almost all real-world applications.
As I understand it, however, sizeof(T) is not the same as sizeof(T[1])
if T is not of a naturally aligning size. The size of svn_txdelta_op_t, for
instance is not necessarily a multiple of 16. If the compiler is instructed
to align data to 16 byte boundaries, the difference becomes important.
Fixed in r987869.
> 5) Got the following error from mod_dav_svn/repos.c:
> ..\..\..\subversion\mod_dav_svn\repos.c(1522): error C4013:
> 'svn_ctype_isspace' undefined; assuming extern returning int
>
> Adding an #include "svn_ctype.h" fixed that, but I'm not too sure if that's ok.
>
>
Committed as r987893.
> After these changes, I can get svn.exe to build. For svnadmin and
> svnserve, I still get linker errors like:
>
> main.obj : error LNK2019: unresolved external symbol
> _svn_fs_fs__set_cache_config referenced in function _main
> main.obj : error LNK2019: unresolved external symbol
> _svn_fs_fs__get_cache_config referenced in function _main
> ..\..\..\Debug\subversion\svnadmin\svnadmin.exe : fatal error LNK1120:
> 2 unresolved externals
>
>
> I haven't been able to resolve these ...
>
Fixed in r987886.
Thanks for the feedback!
-- Stefan^2.
RE: Performance branch ready for review
Posted by Bert Huijben <be...@vmoo.com>.
> -----Original Message-----
> From: Johan Corveleyn [mailto:jcorvel@gmail.com]
> Sent: woensdag 18 augustus 2010 15:59
> To: Stefan Fuhrmann
> Cc: Subversion Development
> Subject: Re: Performance branch ready for review
>
> On Wed, Aug 18, 2010 at 9:14 PM, Stefan Fuhrmann
> <st...@alice-dsl.de> wrote:
> > Hi @all,
> >
> > I just finished my porting work; the performance branch
> > is now fully synchronized with my prototype code.
> > From my point of view, review can start now.
> >
> > According to my measurements, the code is now faster
> > than the original prototype. Large caches provided, a
> > single multi-threaded svnserve instance on a modern
> > quad-core machine should be able to saturate a 10Gb
> > server connection.
> >
> > Open issues / things still to do
> >
> > * there is an issue with "log" triggering an assertion()
> > -> I will investigate that next
> > * test mod_web_dav and add FSFS cache configuration
> > parameters to it
> > * tune membuffer cache eviction strategy such that even
> > small caches can have a large impact
> > * add tests for the new APIs
> > * provide APR patches.
> >
> > There are many things I would like to do but they may
> > better be deferred to 1.8.
>
> I tried compiling your branch on Windows (XP) with Visual C++ Express
> 2008 (which I also use successfully to build trunk). I had a couple of
> issues. FWIW I'm listing them here. I'm not an expert, just
> pragmatically trying to get the thing to build, so some of these
> things may be "user error". Eventually, I was able to build svn.exe,
> but svnadmin.exe and svnserve.exe still fail for me.
>
> 1) In build.conf, I added private\svn_temp_serializer.h and
> private\svn_file_handle_cache.h to the list of msvc-export of
> libsvn_subr. Otherwise, the linker gave problems:
>
> libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
> symbol _svn_file_handle_cache__has_file referenced in function
> _svn_fs_fs__path_rev_absolute
> libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
> symbol _svn_stream__from_cached_file_handle referenced in function
> _get_node_revision_body
> libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
> symbol _svn_file_handle_cache__open referenced in function
> _get_node_revision_body
> libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
> symbol _svn_file_handle_cache__get_apr_handle referenced in function
> _open_pack_or_rev_file
> libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
> symbol _svn_file_handle_cache__flush referenced in function
> _sync_file_handle_cache
> libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
> symbol _svn_file_handle_cache__close referenced in function
> _svn_fs_fs__rev_get_root
> libsvn_fs_fs-1.lib(caching.obj) : error LNK2019: unresolved external
> symbol _svn_file_handle_cache__create_cache referenced in function
> _get_global_file_handle_cache
> libsvn_fs_fs-1.lib(id.obj) : error LNK2019: unresolved external symbol
> _svn_temp_serializer__pop referenced in function
> _svn_fs_fs__id_serialize
> libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2001: unresolved
> external symbol _svn_temp_serializer__pop
> libsvn_fs_fs-1.lib(id.obj) : error LNK2019: unresolved external symbol
> _svn_temp_serializer__push referenced in function
> _svn_fs_fs__id_serialize
> libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2001: unresolved
> external symbol _svn_temp_serializer__push
> libsvn_fs_fs-1.lib(id.obj) : error LNK2019: unresolved external symbol
> _svn_temp_serializer__add_string referenced in function
> _serialize_id_private
> libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2001: unresolved
> external symbol _svn_temp_serializer__add_string
> libsvn_fs_fs-1.lib(dag.obj) : error LNK2001: unresolved external
> symbol _svn_temp_serializer__add_string
> libsvn_fs_fs-1.lib(id.obj) : error LNK2019: unresolved external symbol
> _svn_temp_deserializer__resolve referenced in function
> _svn_fs_fs__id_deserialize
> libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2001: unresolved
> external symbol _svn_temp_deserializer__resolve
> libsvn_fs_fs-1.lib(dag.obj) : error LNK2001: unresolved external
> symbol _svn_temp_deserializer__resolve
> libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2019: unresolved
> external symbol _svn_temp_serializer__get referenced in function
> _svn_fs_fs__serialize_txdelta_window
> libsvn_fs_fs-1.lib(dag.obj) : error LNK2001: unresolved external
> symbol _svn_temp_serializer__get
> libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2019: unresolved
> external symbol _svn_temp_serializer__init referenced in function
> _svn_fs_fs__serialize_txdelta_window
> libsvn_fs_fs-1.lib(dag.obj) : error LNK2001: unresolved external
> symbol _svn_temp_serializer__init
> libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2019: unresolved
> external symbol _svn_temp_deserializer__ptr referenced in function
> _svn_fs_fs__extract_dir_entry
> libsvn_fs_fs-1.lib(dag.obj) : error LNK2019: unresolved external
> symbol _svn_temp_serializer__set_null referenced in function
> _svn_fs_fs__dag_serialize
> ..\..\..\Debug\subversion\libsvn_fs\libsvn_fs-1.dll : fatal error
> LNK1120: 15 unresolved externals
>
> Here's a patch that solves this for me:
> [[[
> Index: build.conf
> ==========================================================
> =========
> --- build.conf (revision 986928)
> +++ build.conf (working copy)
> @@ -322,6 +322,7 @@
> private\svn_opt_private.h private\svn_skel.h private\svn_sqlite.h
> private\svn_utf_private.h private\svn_eol_private.h
> private\svn_token.h
> + private\svn_temp_serializer.h private\svn_file_handle_cache.h
>
> # Working copy management lib
> [libsvn_wc]
> ]]]
Looks like the right fix.
> 3) In function store_current_end_pointer in svn_temp_serializer_c,
> target_ptr needs to be declared before the statements.
Yes, this is required for C89 compatibility. (And VC is configured to check
for that even though it normally allows this case)
> 4) In temp_serializer.c, some of the sizeof statements gave problems for
me:
>
> ..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(274): error C2057:
> expected constant expression
> ..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(274): error C2466:
> cannot allocate an array of constant size 0
> ..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(274): warning
> C4034: sizeof returns 0
> ..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(424): error C2057:
> expected constant expression
> ..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(424): error C2466:
> cannot allocate an array of constant size 0
> ..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(424): warning
> C4034: sizeof returns 0
>
> I changed them as follows (not sure if this is correct, but this compiles
ok):
> [[[
> Index: subversion/libsvn_fs_fs/temp_serializer.c
> ==========================================================
> =========
> --- subversion/libsvn_fs_fs/temp_serializer.c (revision 986928)
> +++ subversion/libsvn_fs_fs/temp_serializer.c (working copy)
> @@ -271,7 +271,7 @@
>
> /* calculate sizes */
> apr_size_t count = apr_hash_count(entries);
> - apr_size_t entries_len = sizeof(svn_fs_dirent_t*[count]);
> + apr_size_t entries_len = sizeof(svn_fs_dirent_t*) * count;
>
> /* copy the hash entries to an auxilliary struct of known layout */
> hash_data.count = count;
> @@ -421,7 +421,7 @@
> /* the ops form a simple chunk of memory with no further references */
> svn_temp_serializer__push(context,
> (const void * const *)ops,
> - sizeof(svn_txdelta_op_t[count]));
> + sizeof(svn_txdelta_op_t) * count);
> svn_temp_serializer__pop(context);
> }
>
> ]]]
Looks ok.
> 5) Got the following error from mod_dav_svn/repos.c:
> ..\..\..\subversion\mod_dav_svn\repos.c(1522): error C4013:
> 'svn_ctype_isspace' undefined; assuming extern returning int
>
> Adding an #include "svn_ctype.h" fixed that, but I'm not too sure if
that's ok.
Should be ok.
> After these changes, I can get svn.exe to build. For svnadmin and
> svnserve, I still get linker errors like:
>
> main.obj : error LNK2019: unresolved external symbol
> _svn_fs_fs__set_cache_config referenced in function _main
> main.obj : error LNK2019: unresolved external symbol
> _svn_fs_fs__get_cache_config referenced in function _main
> ..\..\..\Debug\subversion\svnadmin\svnadmin.exe : fatal error LNK1120:
> 2 unresolved externals
I think this requires adding an additional dependency to svnadmin in
build.conf.
(My guess would be libsvn_fs_fs; but I can't look at the code right now)
@Stefan, can you apply these changes?
Thanks,
Bert
Re: Performance branch ready for review
Posted by Johan Corveleyn <jc...@gmail.com>.
On Wed, Aug 18, 2010 at 9:14 PM, Stefan Fuhrmann
<st...@alice-dsl.de> wrote:
> Hi @all,
>
> I just finished my porting work; the performance branch
> is now fully synchronized with my prototype code.
> From my point of view, review can start now.
>
> According to my measurements, the code is now faster
> than the original prototype. Large caches provided, a
> single multi-threaded svnserve instance on a modern
> quad-core machine should be able to saturate a 10Gb
> server connection.
>
> Open issues / things still to do
>
> * there is an issue with "log" triggering an assertion()
> -> I will investigate that next
> * test mod_web_dav and add FSFS cache configuration
> parameters to it
> * tune membuffer cache eviction strategy such that even
> small caches can have a large impact
> * add tests for the new APIs
> * provide APR patches.
>
> There are many things I would like to do but they may
> better be deferred to 1.8.
I tried compiling your branch on Windows (XP) with Visual C++ Express
2008 (which I also use successfully to build trunk). I had a couple of
issues. FWIW I'm listing them here. I'm not an expert, just
pragmatically trying to get the thing to build, so some of these
things may be "user error". Eventually, I was able to build svn.exe,
but svnadmin.exe and svnserve.exe still fail for me.
1) In build.conf, I added private\svn_temp_serializer.h and
private\svn_file_handle_cache.h to the list of msvc-export of
libsvn_subr. Otherwise, the linker gave problems:
libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
symbol _svn_file_handle_cache__has_file referenced in function
_svn_fs_fs__path_rev_absolute
libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
symbol _svn_stream__from_cached_file_handle referenced in function
_get_node_revision_body
libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
symbol _svn_file_handle_cache__open referenced in function
_get_node_revision_body
libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
symbol _svn_file_handle_cache__get_apr_handle referenced in function
_open_pack_or_rev_file
libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
symbol _svn_file_handle_cache__flush referenced in function
_sync_file_handle_cache
libsvn_fs_fs-1.lib(fs_fs.obj) : error LNK2019: unresolved external
symbol _svn_file_handle_cache__close referenced in function
_svn_fs_fs__rev_get_root
libsvn_fs_fs-1.lib(caching.obj) : error LNK2019: unresolved external
symbol _svn_file_handle_cache__create_cache referenced in function
_get_global_file_handle_cache
libsvn_fs_fs-1.lib(id.obj) : error LNK2019: unresolved external symbol
_svn_temp_serializer__pop referenced in function
_svn_fs_fs__id_serialize
libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2001: unresolved
external symbol _svn_temp_serializer__pop
libsvn_fs_fs-1.lib(id.obj) : error LNK2019: unresolved external symbol
_svn_temp_serializer__push referenced in function
_svn_fs_fs__id_serialize
libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2001: unresolved
external symbol _svn_temp_serializer__push
libsvn_fs_fs-1.lib(id.obj) : error LNK2019: unresolved external symbol
_svn_temp_serializer__add_string referenced in function
_serialize_id_private
libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2001: unresolved
external symbol _svn_temp_serializer__add_string
libsvn_fs_fs-1.lib(dag.obj) : error LNK2001: unresolved external
symbol _svn_temp_serializer__add_string
libsvn_fs_fs-1.lib(id.obj) : error LNK2019: unresolved external symbol
_svn_temp_deserializer__resolve referenced in function
_svn_fs_fs__id_deserialize
libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2001: unresolved
external symbol _svn_temp_deserializer__resolve
libsvn_fs_fs-1.lib(dag.obj) : error LNK2001: unresolved external
symbol _svn_temp_deserializer__resolve
libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2019: unresolved
external symbol _svn_temp_serializer__get referenced in function
_svn_fs_fs__serialize_txdelta_window
libsvn_fs_fs-1.lib(dag.obj) : error LNK2001: unresolved external
symbol _svn_temp_serializer__get
libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2019: unresolved
external symbol _svn_temp_serializer__init referenced in function
_svn_fs_fs__serialize_txdelta_window
libsvn_fs_fs-1.lib(dag.obj) : error LNK2001: unresolved external
symbol _svn_temp_serializer__init
libsvn_fs_fs-1.lib(temp_serializer.obj) : error LNK2019: unresolved
external symbol _svn_temp_deserializer__ptr referenced in function
_svn_fs_fs__extract_dir_entry
libsvn_fs_fs-1.lib(dag.obj) : error LNK2019: unresolved external
symbol _svn_temp_serializer__set_null referenced in function
_svn_fs_fs__dag_serialize
..\..\..\Debug\subversion\libsvn_fs\libsvn_fs-1.dll : fatal error
LNK1120: 15 unresolved externals
Here's a patch that solves this for me:
[[[
Index: build.conf
===================================================================
--- build.conf (revision 986928)
+++ build.conf (working copy)
@@ -322,6 +322,7 @@
private\svn_opt_private.h private\svn_skel.h private\svn_sqlite.h
private\svn_utf_private.h private\svn_eol_private.h
private\svn_token.h
+ private\svn_temp_serializer.h private\svn_file_handle_cache.h
# Working copy management lib
[libsvn_wc]
]]]
2) The NULL in svn_temp_serializer__set_NULL in svn_temp_serializer_h
also gave the linker a hard time:
libsvn_fs_fs-1.lib(dag.obj) : error LNK2019: unresolved external
symbol _svn_temp_serializer__set_NULL referenced in function
_svn_fs_fs__dag_serialize
..\..\..\Debug\subversion\libsvn_fs\libsvn_fs-1.dll : fatal error
LNK1120: 1 unresolved externals
Changing the NULL to null (and updating callers) fixed that.
3) In function store_current_end_pointer in svn_temp_serializer_c,
target_ptr needs to be declared before the statements.
4) In temp_serializer.c, some of the sizeof statements gave problems for me:
..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(274): error C2057:
expected constant expression
..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(274): error C2466:
cannot allocate an array of constant size 0
..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(274): warning
C4034: sizeof returns 0
..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(424): error C2057:
expected constant expression
..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(424): error C2466:
cannot allocate an array of constant size 0
..\..\..\subversion\libsvn_fs_fs\temp_serializer.c(424): warning
C4034: sizeof returns 0
I changed them as follows (not sure if this is correct, but this compiles ok):
[[[
Index: subversion/libsvn_fs_fs/temp_serializer.c
===================================================================
--- subversion/libsvn_fs_fs/temp_serializer.c (revision 986928)
+++ subversion/libsvn_fs_fs/temp_serializer.c (working copy)
@@ -271,7 +271,7 @@
/* calculate sizes */
apr_size_t count = apr_hash_count(entries);
- apr_size_t entries_len = sizeof(svn_fs_dirent_t*[count]);
+ apr_size_t entries_len = sizeof(svn_fs_dirent_t*) * count;
/* copy the hash entries to an auxilliary struct of known layout */
hash_data.count = count;
@@ -421,7 +421,7 @@
/* the ops form a simple chunk of memory with no further references */
svn_temp_serializer__push(context,
(const void * const *)ops,
- sizeof(svn_txdelta_op_t[count]));
+ sizeof(svn_txdelta_op_t) * count);
svn_temp_serializer__pop(context);
}
]]]
5) Got the following error from mod_dav_svn/repos.c:
..\..\..\subversion\mod_dav_svn\repos.c(1522): error C4013:
'svn_ctype_isspace' undefined; assuming extern returning int
Adding an #include "svn_ctype.h" fixed that, but I'm not too sure if that's ok.
After these changes, I can get svn.exe to build. For svnadmin and
svnserve, I still get linker errors like:
main.obj : error LNK2019: unresolved external symbol
_svn_fs_fs__set_cache_config referenced in function _main
main.obj : error LNK2019: unresolved external symbol
_svn_fs_fs__get_cache_config referenced in function _main
..\..\..\Debug\subversion\svnadmin\svnadmin.exe : fatal error LNK1120:
2 unresolved externals
I haven't been able to resolve these ...
Cheers,
--
Johan