You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2013/02/04 11:38:46 UTC

svn commit: r1442071 - in /subversion/trunk/subversion/libsvn_fs_fs: dag.c temp_serializer.c

Author: stefan2
Date: Mon Feb  4 10:38:45 2013
New Revision: 1442071

URL: http://svn.apache.org/viewvc?rev=1442071&view=rev
Log:
Speed up serialization of DAG and noderev structures for our caches.
Turns out that 503 bytes is often not sufficient for noderevs with
longer path names and even less so for DAG nodes.  Up that to 1007
bytes (+1 0-terminator +16 bytes memory management overhead = 1kB).

* subversion/libsvn_fs_fs/dag.c
  (svn_fs_fs__dag_serialize): bump default buffer size

* subversion/libsvn_fs_fs/temp_serializer.c
  (svn_fs_fs__serialize_node_revision): ditto

Modified:
    subversion/trunk/subversion/libsvn_fs_fs/dag.c
    subversion/trunk/subversion/libsvn_fs_fs/temp_serializer.c

Modified: subversion/trunk/subversion/libsvn_fs_fs/dag.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c?rev=1442071&r1=1442070&r2=1442071&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/dag.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/dag.c Mon Feb  4 10:38:45 2013
@@ -1095,7 +1095,7 @@ svn_fs_fs__dag_serialize(void **data,
   svn_temp_serializer__context_t *context =
       svn_temp_serializer__init(node,
                                 sizeof(*node),
-                                503,
+                                1007,
                                 pool);
 
   /* for mutable nodes, we will _never_ cache the noderev */

Modified: subversion/trunk/subversion/libsvn_fs_fs/temp_serializer.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/temp_serializer.c?rev=1442071&r1=1442070&r2=1442071&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/temp_serializer.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/temp_serializer.c Mon Feb  4 10:38:45 2013
@@ -741,7 +741,7 @@ svn_fs_fs__serialize_node_revision(void 
 
   /* create an (empty) serialization context with plenty of buffer space */
   svn_temp_serializer__context_t *context =
-      svn_temp_serializer__init(NULL, 0, 503, pool);
+      svn_temp_serializer__init(NULL, 0, 1007, pool);
 
   /* serialize the noderev */
   svn_fs_fs__noderev_serialize(context, &noderev);



Re: svn commit: r1442071 - in /subversion/trunk/subversion/libsvn_fs_fs: dag.c temp_serializer.c

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Mon, Feb 4, 2013 at 7:31 PM, Blair Zajac <bl...@orcaware.com> wrote:

> On 02/04/2013 02:38 AM, stefan2@apache.org wrote:
>
>> Author: stefan2
>> Date: Mon Feb  4 10:38:45 2013
>> New Revision: 1442071
>>
>> URL: http://svn.apache.org/viewvc?**rev=1442071&view=rev<http://svn.apache.org/viewvc?rev=1442071&view=rev>
>> Log:
>> Speed up serialization of DAG and noderev structures for our caches.
>> Turns out that 503 bytes is often not sufficient for noderevs with
>> longer path names and even less so for DAG nodes.  Up that to 1007
>> bytes (+1 0-terminator +16 bytes memory management overhead = 1kB).
>>
>
> What happens with paths that are longer than 1007 bytes?  If two paths
> differ, say at the suffix, past that length, then will they have the same
> cache key?
>

The buffer will be auto-expanded (see docstring).
But that comes at a non-zero extra cost, so let's
avoid it in the majority of cases.

-- Stefan^2.

-- 
Certified & Supported Apache Subversion Downloads:
*

http://www.wandisco.com/subversion/download
*

Re: svn commit: r1442071 - in /subversion/trunk/subversion/libsvn_fs_fs: dag.c temp_serializer.c

Posted by Blair Zajac <bl...@orcaware.com>.
On 02/04/2013 02:38 AM, stefan2@apache.org wrote:
> Author: stefan2
> Date: Mon Feb  4 10:38:45 2013
> New Revision: 1442071
>
> URL: http://svn.apache.org/viewvc?rev=1442071&view=rev
> Log:
> Speed up serialization of DAG and noderev structures for our caches.
> Turns out that 503 bytes is often not sufficient for noderevs with
> longer path names and even less so for DAG nodes.  Up that to 1007
> bytes (+1 0-terminator +16 bytes memory management overhead = 1kB).

What happens with paths that are longer than 1007 bytes?  If two paths 
differ, say at the suffix, past that length, then will they have the 
same cache key?

Blair


Re: svn commit: r1442071 - in /subversion/trunk/subversion/libsvn_fs_fs: dag.c temp_serializer.c

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Mon, Feb 4, 2013 at 2:02 PM, Philip Martin <ph...@wandisco.com>wrote:

> stefan2@apache.org writes:
>
> > Author: stefan2
> > Date: Mon Feb  4 10:38:45 2013
> > New Revision: 1442071
> >
> > URL: http://svn.apache.org/viewvc?rev=1442071&view=rev
> > Log:
> > Speed up serialization of DAG and noderev structures for our caches.
> > Turns out that 503 bytes is often not sufficient for noderevs with
> > longer path names and even less so for DAG nodes.  Up that to 1007
> > bytes (+1 0-terminator +16 bytes memory management overhead = 1kB).
>
> These comments about the size would be better in the code.  Perhaps this
> number should be a named constant?
>

Done in r1448587.

Thanks for the review!

-- Stefan^2,

-- 
Certified & Supported Apache Subversion Downloads:
*

http://www.wandisco.com/subversion/download
*

Re: svn commit: r1442071 - in /subversion/trunk/subversion/libsvn_fs_fs: dag.c temp_serializer.c

Posted by Philip Martin <ph...@wandisco.com>.
stefan2@apache.org writes:

> Author: stefan2
> Date: Mon Feb  4 10:38:45 2013
> New Revision: 1442071
>
> URL: http://svn.apache.org/viewvc?rev=1442071&view=rev
> Log:
> Speed up serialization of DAG and noderev structures for our caches.
> Turns out that 503 bytes is often not sufficient for noderevs with
> longer path names and even less so for DAG nodes.  Up that to 1007
> bytes (+1 0-terminator +16 bytes memory management overhead = 1kB).

These comments about the size would be better in the code.  Perhaps this
number should be a named constant?

> --- subversion/trunk/subversion/libsvn_fs_fs/dag.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/dag.c Mon Feb  4 10:38:45 2013
> @@ -1095,7 +1095,7 @@ svn_fs_fs__dag_serialize(void **data,
>    svn_temp_serializer__context_t *context =
>        svn_temp_serializer__init(node,
>                                  sizeof(*node),
> -                                503,
> +                                1007,
>                                  pool);
>  
>    /* for mutable nodes, we will _never_ cache the noderev */

> --- subversion/trunk/subversion/libsvn_fs_fs/temp_serializer.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/temp_serializer.c Mon Feb  4 10:38:45 2013
> @@ -741,7 +741,7 @@ svn_fs_fs__serialize_node_revision(void 
>  
>    /* create an (empty) serialization context with plenty of buffer space */
>    svn_temp_serializer__context_t *context =
> -      svn_temp_serializer__init(NULL, 0, 503, pool);
> +      svn_temp_serializer__init(NULL, 0, 1007, pool);
>  
>    /* serialize the noderev */
>    svn_fs_fs__noderev_serialize(context, &noderev);
>
>

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download