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/12/07 10:30:42 UTC
svn commit: r1548823 - in /subversion/trunk/subversion/libsvn_fs_x: dag.c
id.c id.h temp_serializer.c temp_serializer.h
Author: stefan2
Date: Sat Dec 7 09:30:42 2013
New Revision: 1548823
URL: http://svn.apache.org/r1548823
Log:
Add a pool member to the internal representation of FSX IDs and
thus allow the ID vtable functions to perform actual data lookups
in the future. The pool is the one used to allocate the ID.
This is a first step towards making FSX IDs leaner and smarter.
* subversion/libsvn_fs_x/id.h
(svn_fs_x__id_deserialize): Add POOL parameter that is already
available to calling functions.
* subversion/libsvn_fs_x/id.c
(fs_x__id_t): Extend internal ID representation.
(svn_fs_x__id_eq): Adjust the amount of data to compare.
(svn_fs_x__id_txn_create_root,
svn_fs_x__id_create_root,
svn_fs_x__id_txn_create,
svn_fs_x__id_rev_create): Initialize the new struct member.
(svn_fs_x__id_copy): Ditto, plus use a more efficient duplication func.
(svn_fs_x__id_parse,
svn_fs_x__id_deserialize): Initialize the new struct member.
* subversion/libsvn_fs_x/temp_serializer.h
(svn_fs_x__noderev_deserialize): Add POOL parameter as pass-through.
* subversion/libsvn_fs_x/temp_serializer.c
(deserialize_dir,
svn_fs_x__noderev_deserialize,
svn_fs_x__deserialize_id,
svn_fs_x__deserialize_node_revision,
svn_fs_x__extract_dir_entry,
deserialize_change,
svn_fs_x__deserialize_changes): Provide everyone who needs it with
a POOL parameter.
* subversion/libsvn_fs_x/dag.c
(svn_fs_x__dag_deserialize): Update API caller.
Modified:
subversion/trunk/subversion/libsvn_fs_x/dag.c
subversion/trunk/subversion/libsvn_fs_x/id.c
subversion/trunk/subversion/libsvn_fs_x/id.h
subversion/trunk/subversion/libsvn_fs_x/temp_serializer.c
subversion/trunk/subversion/libsvn_fs_x/temp_serializer.h
Modified: subversion/trunk/subversion/libsvn_fs_x/dag.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/dag.c?rev=1548823&r1=1548822&r2=1548823&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/dag.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/dag.c Sat Dec 7 09:30:42 2013
@@ -1152,10 +1152,11 @@ svn_fs_x__dag_deserialize(void **out,
node->fs = NULL;
/* fixup all references to sub-structures */
- svn_fs_x__id_deserialize(node, &node->id);
+ svn_fs_x__id_deserialize(node, &node->id, pool);
svn_fs_x__id_deserialize(node,
- (svn_fs_id_t **)&node->fresh_root_predecessor_id);
- svn_fs_x__noderev_deserialize(node, &node->node_revision);
+ (svn_fs_id_t **)&node->fresh_root_predecessor_id,
+ pool);
+ svn_fs_x__noderev_deserialize(node, &node->node_revision, pool);
node->node_pool = pool;
svn_temp_deserializer__resolve(node, (void**)&node->created_path);
Modified: subversion/trunk/subversion/libsvn_fs_x/id.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/id.c?rev=1548823&r1=1548822&r2=1548823&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/id.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/id.c Sat Dec 7 09:30:42 2013
@@ -41,6 +41,8 @@ typedef struct fs_x__id_t
svn_fs_x__id_part_t copy_id;
svn_fs_x__id_part_t txn_id;
svn_fs_x__id_part_t rev_item;
+
+ apr_pool_t *pool; /* pool that was used to allocate this struct */
} fs_x__id_t;
@@ -286,7 +288,7 @@ svn_fs_x__id_eq(const svn_fs_id_t *a,
return TRUE;
return memcmp(&id_a->node_id, &id_b->node_id,
- sizeof(*id_a) - sizeof(id_a->generic_id)) == 0;
+ 4 * sizeof(svn_fs_x__id_part_t)) == 0;
}
@@ -356,6 +358,7 @@ svn_fs_x__id_txn_create_root(const svn_f
id->generic_id.vtable = &id_vtable;
id->generic_id.fsap_data = &id;
+ id->pool = pool;
return (svn_fs_id_t *)id;
}
@@ -371,6 +374,7 @@ svn_fs_id_t *svn_fs_x__id_create_root(co
id->generic_id.vtable = &id_vtable;
id->generic_id.fsap_data = &id;
+ id->pool = pool;
return (svn_fs_id_t *)id;
}
@@ -390,6 +394,7 @@ svn_fs_x__id_txn_create(const svn_fs_x__
id->generic_id.vtable = &id_vtable;
id->generic_id.fsap_data = &id;
+ id->pool = pool;
return (svn_fs_id_t *)id;
}
@@ -410,6 +415,7 @@ svn_fs_x__id_rev_create(const svn_fs_x__
id->generic_id.vtable = &id_vtable;
id->generic_id.fsap_data = &id;
+ id->pool = pool;
return (svn_fs_id_t *)id;
}
@@ -419,10 +425,10 @@ svn_fs_id_t *
svn_fs_x__id_copy(const svn_fs_id_t *source, apr_pool_t *pool)
{
fs_x__id_t *id = (fs_x__id_t *)source;
- fs_x__id_t *new_id = apr_palloc(pool, sizeof(*new_id));
+ fs_x__id_t *new_id = apr_pmemdup(pool, id, sizeof(*id));
- *new_id = *id;
new_id->generic_id.fsap_data = new_id;
+ new_id->pool = pool;
return (svn_fs_id_t *)new_id;
}
@@ -444,6 +450,7 @@ svn_fs_x__id_parse(const char *data,
id = apr_pcalloc(pool, sizeof(*id));
id->generic_id.vtable = &id_vtable;
id->generic_id.fsap_data = &id;
+ id->pool = pool;
/* Now, we basically just need to "split" this data on `.'
characters. We will use svn_cstring_tokenize, which will put
@@ -531,7 +538,9 @@ svn_fs_x__id_serialize(svn_temp_serializ
/* Deserialize an ID inside the BUFFER.
*/
void
-svn_fs_x__id_deserialize(void *buffer, svn_fs_id_t **in_out)
+svn_fs_x__id_deserialize(void *buffer,
+ svn_fs_id_t **in_out,
+ apr_pool_t *pool)
{
fs_x__id_t *id;
@@ -549,5 +558,6 @@ svn_fs_x__id_deserialize(void *buffer, s
/* the stored vtable is bogus at best -> set the right one */
id->generic_id.vtable = &id_vtable;
id->generic_id.fsap_data = id;
+ id->pool = pool;
}
Modified: subversion/trunk/subversion/libsvn_fs_x/id.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/id.h?rev=1548823&r1=1548822&r2=1548823&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/id.h (original)
+++ subversion/trunk/subversion/libsvn_fs_x/id.h Sat Dec 7 09:30:42 2013
@@ -161,11 +161,12 @@ svn_fs_x__id_serialize(struct svn_temp_s
const svn_fs_id_t * const *id);
/**
- * Deserialize an @a id within the @a buffer.
+ * Deserialize an @a id within the @a buffer and associate it with @a pool.
*/
void
svn_fs_x__id_deserialize(void *buffer,
- svn_fs_id_t **id);
+ svn_fs_id_t **id,
+ apr_pool_t *pool);
#ifdef __cplusplus
}
Modified: subversion/trunk/subversion/libsvn_fs_x/temp_serializer.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/temp_serializer.c?rev=1548823&r1=1548822&r2=1548823&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/temp_serializer.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/temp_serializer.c Sat Dec 7 09:30:42 2013
@@ -324,7 +324,7 @@ deserialize_dir(void *buffer, hash_data_
/* pointer fixup */
svn_temp_deserializer__resolve(entry, (void **)&entry->name);
- svn_fs_x__id_deserialize(entry, (svn_fs_id_t **)&entry->id);
+ svn_fs_x__id_deserialize(entry, (svn_fs_id_t **)&entry->id, pool);
/* add the entry to the hash */
svn_hash_sets(result, entry->name, entry);
@@ -364,7 +364,8 @@ svn_fs_x__noderev_serialize(svn_temp_ser
void
svn_fs_x__noderev_deserialize(void *buffer,
- node_revision_t **noderev_p)
+ node_revision_t **noderev_p,
+ apr_pool_t *pool)
{
node_revision_t *noderev;
@@ -378,8 +379,10 @@ svn_fs_x__noderev_deserialize(void *buff
return;
/* fixup of sub-structures */
- svn_fs_x__id_deserialize(noderev, (svn_fs_id_t **)&noderev->id);
- svn_fs_x__id_deserialize(noderev, (svn_fs_id_t **)&noderev->predecessor_id);
+ svn_fs_x__id_deserialize(noderev, (svn_fs_id_t **)&noderev->id, pool);
+ svn_fs_x__id_deserialize(noderev,
+ (svn_fs_id_t **)&noderev->predecessor_id,
+ pool);
svn_temp_deserializer__resolve(noderev, (void **)&noderev->prop_rep);
svn_temp_deserializer__resolve(noderev, (void **)&noderev->data_rep);
@@ -687,7 +690,7 @@ svn_fs_x__deserialize_id(void **out,
svn_fs_id_t *id = (svn_fs_id_t *)data;
/* fixup of all pointers etc. */
- svn_fs_x__id_deserialize(id, &id);
+ svn_fs_x__id_deserialize(id, &id, pool);
/* done */
*out = id;
@@ -733,7 +736,7 @@ svn_fs_x__deserialize_node_revision(void
node_revision_t *noderev = (node_revision_t *)buffer;
/* fixup of all pointers etc. */
- svn_fs_x__noderev_deserialize(noderev, &noderev);
+ svn_fs_x__noderev_deserialize(noderev, &noderev, pool);
/* done */
*item = noderev;
@@ -891,7 +894,8 @@ svn_fs_x__extract_dir_entry(void **out,
memcpy(new_entry, source, size);
svn_temp_deserializer__resolve(new_entry, (void **)&new_entry->name);
- svn_fs_x__id_deserialize(new_entry, (svn_fs_id_t **)&new_entry->id);
+ svn_fs_x__id_deserialize(new_entry, (svn_fs_id_t **)&new_entry->id,
+ pool);
*(svn_fs_dirent_t **)out = new_entry;
}
@@ -1085,7 +1089,9 @@ serialize_change(svn_temp_serializer__co
* serialization CONTEXT.
*/
static void
-deserialize_change(void *buffer, change_t **change_p)
+deserialize_change(void *buffer,
+ change_t **change_p,
+ apr_pool_t *pool)
{
change_t * change;
@@ -1097,7 +1103,9 @@ deserialize_change(void *buffer, change_
return;
/* fix-up of sub-structures */
- svn_fs_x__id_deserialize(change, (svn_fs_id_t **)&change->info.node_rev_id);
+ svn_fs_x__id_deserialize(change,
+ (svn_fs_id_t **)&change->info.node_rev_id,
+ pool);
svn_temp_deserializer__resolve(change, (void **)&change->path.data);
svn_temp_deserializer__resolve(change, (void **)&change->info.copyfrom_path);
@@ -1177,7 +1185,8 @@ svn_fs_x__deserialize_changes(void **out
for (i = 0; i < changes->count; ++i)
{
deserialize_change((void*)changes->changes,
- (change_t **)&changes->changes[i]);
+ (change_t **)&changes->changes[i],
+ pool);
APR_ARRAY_PUSH(array, change_t *) = changes->changes[i];
}
Modified: subversion/trunk/subversion/libsvn_fs_x/temp_serializer.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/temp_serializer.h?rev=1548823&r1=1548822&r2=1548823&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/temp_serializer.h (original)
+++ subversion/trunk/subversion/libsvn_fs_x/temp_serializer.h Sat Dec 7 09:30:42 2013
@@ -43,11 +43,13 @@ svn_fs_x__noderev_serialize(struct svn_t
node_revision_t * const *noderev_p);
/**
- * Deserialize a @a noderev_p within the @a buffer.
+ * Deserialize a @a noderev_p within the @a buffer and associate it with
+ * @a pool.
*/
void
svn_fs_x__noderev_deserialize(void *buffer,
- node_revision_t **noderev_p);
+ node_revision_t **noderev_p,
+ apr_pool_t *pool);
/**
* Serialize APR array @a *a within the serialization @a context.
Re: svn commit: r1548823 - in /subversion/trunk/subversion/libsvn_fs_x:
dag.c id.c id.h temp_serializer.c temp_serializer.h
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Sun, Dec 22, 2013 at 2:46 PM, Branko Čibej <br...@wandisco.com> wrote:
> On 22.12.2013 13:54, Stefan Fuhrmann wrote:
>
> [Back from enjoying the plague]
>
> On Sun, Dec 8, 2013 at 4:53 PM, Branko Čibej <br...@wandisco.com> wrote:
>
>> On 08.12.2013 11:52, Stefan Fuhrmann wrote:
>>
>> On Sun, Dec 8, 2013 at 11:27 AM, Daniel Shahaf <d....@daniel.shahaf.name>wrote:
>>
>>> Stefan Fuhrmann wrote on Sun, Dec 08, 2013 at 11:09:47 +0100:> >
>>> >
>>> > Duly noted for Subversion 2.0
>>> >
>>> > For now, however, we have to implement id_vtable_t.
>>>
>>> Huh? id_vtable_t is not a public API. I assume the id vtable falls
>>> under the same rules as the fs_library vtable --- see
>>> fs_library_vtable_t.get_version's docstring.
>>>
>>
>> True, but the following API simply don't provide any more information:
>>
>> svn_fs_check_related(const svn_fs_id_t *a, const svn_fs_id_t *b)
>> svn_fs_compare_ids(const svn_fs_id_t *a, const svn_fs_id_t *b)
>>
>> So, our private vtable depends on the data provided by public API.
>> Adding the pool member to the internal struct is exactly the way to
>> decouple the implementation from the existing public interface.
>>
>>
>> There's nothing wrong with revising the public API to require a scratch
>> pool. Sure, the deprecated, pool-less version will be less efficient. But
>> I've yet to hear why "lean and smart", i.e., unstructured IDs are so much
>> better. than what we have currently. After all, their structure matches the
>> semantics of our repository model, regardless of implementation.
>>
>
> This is not about structured vs. unstructured IDs. It's about needlessly
> exposing internal representation at the API level. For instance, it
> requires
> us to never change the internal node_id assignment rules (copy vs.
> branch):
> svn_fs_compare_ids does not provide a pool argument; hence the ID struct
> needs to always carry all information.
>
>
> I'll just point out that the API does not dictate what kind of IDs you use
> internally in the implementation. The fact that svn_fs_id_t exists does not
> mean you have to use its fixed-size, binary representation in directory
> reps, or anywhere else in the storage layer.
>
I fully agree. That is the whole point of the initial commit.
I suppose you support that one now?
> So I wonder who's confusing API with implementation now.
>
Well, adding a pool member to the ID *implementation* struct
has become necessary due to the limitations of the ID *API*.
-- Stefan^2.
Re: svn commit: r1548823 - in /subversion/trunk/subversion/libsvn_fs_x:
dag.c id.c id.h temp_serializer.c temp_serializer.h
Posted by Branko Čibej <br...@wandisco.com>.
On 22.12.2013 13:54, Stefan Fuhrmann wrote:
> [Back from enjoying the plague]
>
> On Sun, Dec 8, 2013 at 4:53 PM, Branko Čibej <brane@wandisco.com
> <ma...@wandisco.com>> wrote:
>
> On 08.12.2013 11:52, Stefan Fuhrmann wrote:
>> On Sun, Dec 8, 2013 at 11:27 AM, Daniel Shahaf
>> <d.s@daniel.shahaf.name <ma...@daniel.shahaf.name>> wrote:
>>
>> Stefan Fuhrmann wrote on Sun, Dec 08, 2013 at 11:09:47 +0100:> >
>> >
>> > Duly noted for Subversion 2.0
>> >
>> > For now, however, we have to implement id_vtable_t.
>>
>> Huh? id_vtable_t is not a public API. I assume the id
>> vtable falls
>> under the same rules as the fs_library vtable --- see
>> fs_library_vtable_t.get_version's docstring.
>>
>>
>> True, but the following API simply don't provide any more
>> information:
>>
>> svn_fs_check_related(const svn_fs_id_t *a, const svn_fs_id_t *b)
>> svn_fs_compare_ids(const svn_fs_id_t *a, const svn_fs_id_t *b)
>>
>> So, our private vtable depends on the data provided by public API.
>> Adding the pool member to the internal struct is exactly the way to
>> decouple the implementation from the existing public interface.
>
> There's nothing wrong with revising the public API to require a
> scratch pool. Sure, the deprecated, pool-less version will be less
> efficient. But I've yet to hear why "lean and smart", i.e.,
> unstructured IDs are so much better. than what we have currently.
> After all, their structure matches the semantics of our repository
> model, regardless of implementation.
>
>
> This is not about structured vs. unstructured IDs. It's about needlessly
> exposing internal representation at the API level. For instance, it
> requires
> us to never change the internal node_id assignment rules (copy vs.
> branch):
> svn_fs_compare_ids does not provide a pool argument; hence the ID struct
> needs to always carry all information.
I'll just point out that the API does not dictate what kind of IDs you
use internally in the implementation. The fact that svn_fs_id_t exists
does not mean you have to use its fixed-size, binary representation in
directory reps, or anywhere else in the storage layer.
So I wonder who's confusing API with implementation now.
-- Brane
--
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com
Re: svn commit: r1548823 - in /subversion/trunk/subversion/libsvn_fs_x:
dag.c id.c id.h temp_serializer.c temp_serializer.h
Posted by Stefan Fuhrmann <st...@wandisco.com>.
[Back from enjoying the plague]
On Sun, Dec 8, 2013 at 4:53 PM, Branko Čibej <br...@wandisco.com> wrote:
> On 08.12.2013 11:52, Stefan Fuhrmann wrote:
>
> On Sun, Dec 8, 2013 at 11:27 AM, Daniel Shahaf <d....@daniel.shahaf.name>wrote:
>
>> Stefan Fuhrmann wrote on Sun, Dec 08, 2013 at 11:09:47 +0100:> >
>> >
>> > Duly noted for Subversion 2.0
>> >
>> > For now, however, we have to implement id_vtable_t.
>>
>> Huh? id_vtable_t is not a public API. I assume the id vtable falls
>> under the same rules as the fs_library vtable --- see
>> fs_library_vtable_t.get_version's docstring.
>>
>
> True, but the following API simply don't provide any more information:
>
> svn_fs_check_related(const svn_fs_id_t *a, const svn_fs_id_t *b)
> svn_fs_compare_ids(const svn_fs_id_t *a, const svn_fs_id_t *b)
>
> So, our private vtable depends on the data provided by public API.
> Adding the pool member to the internal struct is exactly the way to
> decouple the implementation from the existing public interface.
>
>
> There's nothing wrong with revising the public API to require a scratch
> pool. Sure, the deprecated, pool-less version will be less efficient. But
> I've yet to hear why "lean and smart", i.e., unstructured IDs are so much
> better. than what we have currently. After all, their structure matches the
> semantics of our repository model, regardless of implementation.
>
This is not about structured vs. unstructured IDs. It's about needlessly
exposing internal representation at the API level. For instance, it
requires
us to never change the internal node_id assignment rules (copy vs. branch):
svn_fs_compare_ids does not provide a pool argument; hence the ID struct
needs to always carry all information.
Despite the fact that it may take a few lookups to construct a single ID
(for each dirent and changed path passed back through the API), those
IDs are already huge. At 80 bytes each, they consume > 50% of the
memory in dirs and changed path lists. So, "lean" stands for "storing the
key only" and "smart" is for "gather derived information when actually
requested".
FSX will eventually (try to) implement your new node tree structure
explicitly modelling nodes, noderevs and copies / branches (semantics TBD)
and their keys / IDs might not be lumped together anymore. In a lazy copy
regime, not all node.copy.ver combinations may exist physically. Right now,
the API (in combination with the "no pools in structs" rule) prevents such
designs.
On Sun, Dec 8, 2013 at 5:05 PM, Branko Čibej <br...@wandisco.com> wrote:
> On 08.12.2013 16:34, Stefan Fuhrmann wrote:
>
> On Sun, Dec 8, 2013 at 1:31 PM, Bert Huijben <be...@qqmail.nl> wrote:
>
>> We revved almost the entire wc api for Subversion 1.7, with almost no
>> breakage for the older wc apis. (8 errata if I remember correctly). I don't
>> see why we can't do this for the fs or ra layer APIs before 2.0. The old
>> public APIs that miss arguments won't be as fast as the newer APIs that
>> have them but... I don't see this as an excuse to use a bad design for the
>> fs layer until 2.0.
>>
>
> Well, the alternative would be to eliminate svn_fs_id_t entirely from
> the API,
> which implies revving svn_fs_dirent_t and svn_fs_path_change2_t plus
> related functions. Simply introducing svn_fs_id2_t in parallel to
> svn_fs_id_t
> would needlessly duplicate a lot of code exactly because the underlying
> representations are incompatible.
>
> I would actually like to remve svn_fs_id_t from the API since it violates
> our general path@rev / path@root pattern (deprecating the old API for
> backward compat). Any objections?
>
> If we go down that route, we may as well begin designing the FSv2 API
> instead. This piecemeal twiddling for the sake of a back-end that we won't
> release for several years yet doesn't make much sense to me.
>
I'll stop exposing implementation details needlessly. The changed paths
APIs had been bumped in the past and bits of the ID API already got
deprecated a long time ago. Cleaning up the remnants will not create
FSv1.5 but a clearer FSv1.
See separate post.
On Sun, Dec 8, 2013 at 5:52 PM, Daniel Shahaf <d....@daniel.shahaf.name>wrote:
> Stefan Fuhrmann wrote on Sun, Dec 08, 2013 at 16:34:43 +0100:
> > I would actually like to remve svn_fs_id_t from the API since it violates
> > our general path@rev / path@root pattern (deprecating the old API for
> > backward compat). Any objections?
>
> Stefan, that's not a reason, that's discrimination. It doesn't actually
> explain why the functionality svn_fs_id_t provides is not needed by us
> or by API consumers.
>
How is discrimination in design matters a bad thing?
As for the why, see separate post.
-- Stefan^2.
Re: svn commit: r1548823 - in /subversion/trunk/subversion/libsvn_fs_x:
dag.c id.c id.h temp_serializer.c temp_serializer.h
Posted by Branko Čibej <br...@wandisco.com>.
On 08.12.2013 11:52, Stefan Fuhrmann wrote:
>
>
>
> On Sun, Dec 8, 2013 at 11:27 AM, Daniel Shahaf <d.s@daniel.shahaf.name
> <ma...@daniel.shahaf.name>> wrote:
>
> Stefan Fuhrmann wrote on Sun, Dec 08, 2013 at 11:09:47 +0100:> >
> >
> > Duly noted for Subversion 2.0
> >
> > For now, however, we have to implement id_vtable_t.
>
> Huh? id_vtable_t is not a public API. I assume the id vtable falls
> under the same rules as the fs_library vtable --- see
> fs_library_vtable_t.get_version's docstring.
>
>
> True, but the following API simply don't provide any more information:
>
> svn_fs_check_related(const svn_fs_id_t *a, const svn_fs_id_t *b)
> svn_fs_compare_ids(const svn_fs_id_t *a, const svn_fs_id_t *b)
>
> So, our private vtable depends on the data provided by public API.
> Adding the pool member to the internal struct is exactly the way to
> decouple the implementation from the existing public interface.
There's nothing wrong with revising the public API to require a scratch
pool. Sure, the deprecated, pool-less version will be less efficient.
But I've yet to hear why "lean and smart", i.e., unstructured IDs are so
much better. than what we have currently. After all, their structure
matches the semantics of our repository model, regardless of implementation.
-- Brane
--
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com
Re: svn commit: r1548823 - in /subversion/trunk/subversion/libsvn_fs_x:
dag.c id.c id.h temp_serializer.c temp_serializer.h
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Sun, Dec 8, 2013 at 11:27 AM, Daniel Shahaf <d....@daniel.shahaf.name>wrote:
> Stefan Fuhrmann wrote on Sun, Dec 08, 2013 at 11:09:47 +0100:> >
> >
> > Duly noted for Subversion 2.0
> >
> > For now, however, we have to implement id_vtable_t.
>
> Huh? id_vtable_t is not a public API. I assume the id vtable falls
> under the same rules as the fs_library vtable --- see
> fs_library_vtable_t.get_version's docstring.
>
True, but the following API simply don't provide any more information:
svn_fs_check_related(const svn_fs_id_t *a, const svn_fs_id_t *b)
svn_fs_compare_ids(const svn_fs_id_t *a, const svn_fs_id_t *b)
So, our private vtable depends on the data provided by public API.
Adding the pool member to the internal struct is exactly the way to
decouple the implementation from the existing public interface.
-- Stefan^2.
Re: svn commit: r1548823 - in
/subversion/trunk/subversion/libsvn_fs_x: dag.c id.c id.h
temp_serializer.c temp_serializer.h
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Fuhrmann wrote on Sun, Dec 08, 2013 at 11:09:47 +0100:
> On Sat, Dec 7, 2013 at 11:00 AM, Bert Huijben <be...@qqmail.nl> wrote:
>
> >
> >
> > > -----Original Message-----
> > > From: stefan2@apache.org [mailto:stefan2@apache.org]
> > > Sent: zaterdag 7 december 2013 10:31
> > > To: commits@subversion.apache.org
> > > Subject: svn commit: r1548823 - in
> > > /subversion/trunk/subversion/libsvn_fs_x: dag.c id.c id.h
> > temp_serializer.c
> > > temp_serializer.h
> > >
> > > Author: stefan2
> > > Date: Sat Dec 7 09:30:42 2013
> > > New Revision: 1548823
> > >
> > > URL: http://svn.apache.org/r1548823
> > > Log:
> > > Add a pool member to the internal representation of FSX IDs and
> > > thus allow the ID vtable functions to perform actual data lookups
> > > in the future. The pool is the one used to allocate the ID.
> >
> > With WC-NG we explicitly stopped adding a pool in this kind of structures
> > for very good reasons. All these pool reference makes it harder and harder
> > to properly allocate results. This pattern is far too sensitive to pool
> > cleanup ordering problems
> >
> > Using proper result and scratch pool (where the scratch pool is always the
> > result pool or a descendant of the result pool or at least a pool that is
> > cleaned up *before* the result pool), makes pool cleanup handling much
> > easier to understand...
Incidentally, this confused me. We *depend* on scratch pools getting
cleared before result pools? Normally, the lifetime of a scratch pool
is explicitly undefined: it is not promised to be cleared right after
the call, or before result_pool, or after result_pool; they simply
promise nothing whatsoever.
Do we have code that assumes otherwise?
> >
>
> Duly noted for Subversion 2.0
>
> For now, however, we have to implement id_vtable_t.
Huh? id_vtable_t is not a public API. I assume the id vtable falls
under the same rules as the fs_library vtable --- see
fs_library_vtable_t.get_version's docstring.
> It offers two
> functions, the first one being an identity check is certainly o.k.
>
> The second one tests the following: "does this noderev(*) ID have a
> common ancestor with that other noderev ID?". Unless you somehow
> attach that information to the ID itself, you probably need a pool to
> perform some kind of data access. But the API does not provide one.
Re: svn commit: r1548823 - in /subversion/trunk/subversion/libsvn_fs_x:
dag.c id.c id.h temp_serializer.c temp_serializer.h
Posted by Branko Čibej <br...@wandisco.com>.
On 08.12.2013 11:09, Stefan Fuhrmann wrote:
> On Sat, Dec 7, 2013 at 11:00 AM, Bert Huijben <bert@qqmail.nl
> <ma...@qqmail.nl>> wrote:
>
>
>
> > -----Original Message-----
> > From: stefan2@apache.org <ma...@apache.org>
> [mailto:stefan2@apache.org <ma...@apache.org>]
> > Sent: zaterdag 7 december 2013 10:31
> > To: commits@subversion.apache.org
> <ma...@subversion.apache.org>
> > Subject: svn commit: r1548823 - in
> > /subversion/trunk/subversion/libsvn_fs_x: dag.c id.c id.h
> temp_serializer.c
> > temp_serializer.h
> >
> > Author: stefan2
> > Date: Sat Dec 7 09:30:42 2013
> > New Revision: 1548823
> >
> > URL: http://svn.apache.org/r1548823
> > Log:
> > Add a pool member to the internal representation of FSX IDs and
> > thus allow the ID vtable functions to perform actual data lookups
> > in the future. The pool is the one used to allocate the ID.
>
> With WC-NG we explicitly stopped adding a pool in this kind of
> structures for very good reasons. All these pool reference makes
> it harder and harder to properly allocate results. This pattern is
> far too sensitive to pool cleanup ordering problems
>
> Using proper result and scratch pool (where the scratch pool is
> always the result pool or a descendant of the result pool or at
> least a pool that is cleaned up *before* the result pool), makes
> pool cleanup handling much easier to understand...
>
>
> Duly noted for Subversion 2.0
>
> For now, however, we have to implement id_vtable_t. It offers two
> functions, the first one being an identity check is certainly o.k.
>
> The second one tests the following: "does this noderev(*) ID have a
> common ancestor with that other noderev ID?". Unless you somehow
> attach that information to the ID itself, you probably need a pool to
> perform some kind of data access. But the API does not provide one.
>
> > This is a first step towards making FSX IDs leaner and smarter.
>
> I would have assumed that an ID is constant... How can a constant
> value be smart?
>
> It can be chosen smartly, but an id is an identifier not code.
>
>
> See above.
>
> I'm trying to make the FSX IDs true IDs with no redundant data
> on them and to separate their internal representation from the API.
A commendable goal, and is there an actual reason for it? Other than
saving another few bytes of storage at the expense of making the code
harder to validate, and the repo harder to debug.
-- Brane
--
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com
Re: svn commit: r1548823 - in /subversion/trunk/subversion/libsvn_fs_x:
dag.c id.c id.h temp_serializer.c temp_serializer.h
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Sat, Dec 7, 2013 at 11:00 AM, Bert Huijben <be...@qqmail.nl> wrote:
>
>
> > -----Original Message-----
> > From: stefan2@apache.org [mailto:stefan2@apache.org]
> > Sent: zaterdag 7 december 2013 10:31
> > To: commits@subversion.apache.org
> > Subject: svn commit: r1548823 - in
> > /subversion/trunk/subversion/libsvn_fs_x: dag.c id.c id.h
> temp_serializer.c
> > temp_serializer.h
> >
> > Author: stefan2
> > Date: Sat Dec 7 09:30:42 2013
> > New Revision: 1548823
> >
> > URL: http://svn.apache.org/r1548823
> > Log:
> > Add a pool member to the internal representation of FSX IDs and
> > thus allow the ID vtable functions to perform actual data lookups
> > in the future. The pool is the one used to allocate the ID.
>
> With WC-NG we explicitly stopped adding a pool in this kind of structures
> for very good reasons. All these pool reference makes it harder and harder
> to properly allocate results. This pattern is far too sensitive to pool
> cleanup ordering problems
>
> Using proper result and scratch pool (where the scratch pool is always the
> result pool or a descendant of the result pool or at least a pool that is
> cleaned up *before* the result pool), makes pool cleanup handling much
> easier to understand...
>
Duly noted for Subversion 2.0
For now, however, we have to implement id_vtable_t. It offers two
functions, the first one being an identity check is certainly o.k.
The second one tests the following: "does this noderev(*) ID have a
common ancestor with that other noderev ID?". Unless you somehow
attach that information to the ID itself, you probably need a pool to
perform some kind of data access. But the API does not provide one.
> This is a first step towards making FSX IDs leaner and smarter.
>
> I would have assumed that an ID is constant... How can a constant value be
> smart?
>
> It can be chosen smartly, but an id is an identifier not code.
>
See above.
I'm trying to make the FSX IDs true IDs with no redundant data
on them and to separate their internal representation from the API.
-- Stefan^2.
(*) Noderevs are the FSFS equivalent of what svn_fs_id_t represents.
The actual FS backend may use a different underlying concept but
must still provide noderev-esque semantics through the vtable.
Re: svn commit: r1548823 - in /subversion/trunk/subversion/libsvn_fs_x:
dag.c id.c id.h temp_serializer.c temp_serializer.h
Posted by Branko Čibej <br...@wandisco.com>.
On 07.12.2013 11:00, Bert Huijben wrote:
>
>> -----Original Message-----
>> From: stefan2@apache.org [mailto:stefan2@apache.org]
>> Sent: zaterdag 7 december 2013 10:31
>> To: commits@subversion.apache.org
>> Subject: svn commit: r1548823 - in
>> /subversion/trunk/subversion/libsvn_fs_x: dag.c id.c id.h temp_serializer.c
>> temp_serializer.h
>>
>> Author: stefan2
>> Date: Sat Dec 7 09:30:42 2013
>> New Revision: 1548823
>>
>> URL: http://svn.apache.org/r1548823
>> Log:
>> Add a pool member to the internal representation of FSX IDs and
>> thus allow the ID vtable functions to perform actual data lookups
>> in the future. The pool is the one used to allocate the ID.
> With WC-NG we explicitly stopped adding a pool in this kind of structures for very good reasons. All these pool reference makes it harder and harder to properly allocate results. This pattern is far too sensitive to pool cleanup ordering problems
>
> Using proper result and scratch pool (where the scratch pool is always the result pool or a descendant of the result pool or at least a pool that is cleaned up *before* the result pool), makes pool cleanup handling much easier to understand...
Couldn't agree more. Structs carrying their own pools around (except in
very limited cases, such as stringbufs and similar) are dangerous.
-- Brane
--
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com
RE: svn commit: r1548823 - in /subversion/trunk/subversion/libsvn_fs_x: dag.c id.c id.h temp_serializer.c temp_serializer.h
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: stefan2@apache.org [mailto:stefan2@apache.org]
> Sent: zaterdag 7 december 2013 10:31
> To: commits@subversion.apache.org
> Subject: svn commit: r1548823 - in
> /subversion/trunk/subversion/libsvn_fs_x: dag.c id.c id.h temp_serializer.c
> temp_serializer.h
>
> Author: stefan2
> Date: Sat Dec 7 09:30:42 2013
> New Revision: 1548823
>
> URL: http://svn.apache.org/r1548823
> Log:
> Add a pool member to the internal representation of FSX IDs and
> thus allow the ID vtable functions to perform actual data lookups
> in the future. The pool is the one used to allocate the ID.
With WC-NG we explicitly stopped adding a pool in this kind of structures for very good reasons. All these pool reference makes it harder and harder to properly allocate results. This pattern is far too sensitive to pool cleanup ordering problems
Using proper result and scratch pool (where the scratch pool is always the result pool or a descendant of the result pool or at least a pool that is cleaned up *before* the result pool), makes pool cleanup handling much easier to understand...
> This is a first step towards making FSX IDs leaner and smarter.
I would have assumed that an ID is constant... How can a constant value be smart?
It can be chosen smartly, but an id is an identifier not code.
> * subversion/libsvn_fs_x/id.h
> (svn_fs_x__id_deserialize): Add POOL parameter that is already
> available to calling functions.
>
> * subversion/libsvn_fs_x/id.c
> (fs_x__id_t): Extend internal ID representation.
> (svn_fs_x__id_eq): Adjust the amount of data to compare.
> (svn_fs_x__id_txn_create_root,
> svn_fs_x__id_create_root,
> svn_fs_x__id_txn_create,
> svn_fs_x__id_rev_create): Initialize the new struct member.
> (svn_fs_x__id_copy): Ditto, plus use a more efficient duplication func.
> (svn_fs_x__id_parse,
> svn_fs_x__id_deserialize): Initialize the new struct member.
>
> * subversion/libsvn_fs_x/temp_serializer.h
> (svn_fs_x__noderev_deserialize): Add POOL parameter as pass-through.
>
> * subversion/libsvn_fs_x/temp_serializer.c
> (deserialize_dir,
> svn_fs_x__noderev_deserialize,
> svn_fs_x__deserialize_id,
> svn_fs_x__deserialize_node_revision,
> svn_fs_x__extract_dir_entry,
> deserialize_change,
> svn_fs_x__deserialize_changes): Provide everyone who needs it with
> a POOL parameter.
>
> * subversion/libsvn_fs_x/dag.c
> (svn_fs_x__dag_deserialize): Update API caller.
>
> Modified:
> subversion/trunk/subversion/libsvn_fs_x/dag.c
> subversion/trunk/subversion/libsvn_fs_x/id.c
> subversion/trunk/subversion/libsvn_fs_x/id.h
> subversion/trunk/subversion/libsvn_fs_x/temp_serializer.c
> subversion/trunk/subversion/libsvn_fs_x/temp_serializer.h
>
> Modified: subversion/trunk/subversion/libsvn_fs_x/dag.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/da
> g.c?rev=1548823&r1=1548822&r2=1548823&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_fs_x/dag.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_x/dag.c Sat Dec 7 09:30:42 2013
> @@ -1152,10 +1152,11 @@ svn_fs_x__dag_deserialize(void **out,
> node->fs = NULL;
>
> /* fixup all references to sub-structures */
> - svn_fs_x__id_deserialize(node, &node->id);
> + svn_fs_x__id_deserialize(node, &node->id, pool);
> svn_fs_x__id_deserialize(node,
> - (svn_fs_id_t **)&node->fresh_root_predecessor_id);
> - svn_fs_x__noderev_deserialize(node, &node->node_revision);
> + (svn_fs_id_t **)&node->fresh_root_predecessor_id,
> + pool);
> + svn_fs_x__noderev_deserialize(node, &node->node_revision, pool);
> node->node_pool = pool;
>
> svn_temp_deserializer__resolve(node, (void**)&node->created_path);
>
> Modified: subversion/trunk/subversion/libsvn_fs_x/id.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/id.c
> ?rev=1548823&r1=1548822&r2=1548823&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_fs_x/id.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_x/id.c Sat Dec 7 09:30:42 2013
> @@ -41,6 +41,8 @@ typedef struct fs_x__id_t
> svn_fs_x__id_part_t copy_id;
> svn_fs_x__id_part_t txn_id;
> svn_fs_x__id_part_t rev_item;
> +
> + apr_pool_t *pool; /* pool that was used to allocate this struct */
> } fs_x__id_t;
>
>
> @@ -286,7 +288,7 @@ svn_fs_x__id_eq(const svn_fs_id_t *a,
> return TRUE;
>
> return memcmp(&id_a->node_id, &id_b->node_id,
> - sizeof(*id_a) - sizeof(id_a->generic_id)) == 0;
> + 4 * sizeof(svn_fs_x__id_part_t)) == 0;
> }
This doesn't look like it properly handles alignment within the structure. (It probably works now, but the old code was far less sensitive for future changes to this struct)
Bert
>
>
> @@ -356,6 +358,7 @@ svn_fs_x__id_txn_create_root(const svn_f
>
> id->generic_id.vtable = &id_vtable;
> id->generic_id.fsap_data = &id;
> + id->pool = pool;
>
> return (svn_fs_id_t *)id;
> }
> @@ -371,6 +374,7 @@ svn_fs_id_t *svn_fs_x__id_create_root(co
>
> id->generic_id.vtable = &id_vtable;
> id->generic_id.fsap_data = &id;
> + id->pool = pool;
>
> return (svn_fs_id_t *)id;
> }
> @@ -390,6 +394,7 @@ svn_fs_x__id_txn_create(const svn_fs_x__
>
> id->generic_id.vtable = &id_vtable;
> id->generic_id.fsap_data = &id;
> + id->pool = pool;
>
> return (svn_fs_id_t *)id;
> }
> @@ -410,6 +415,7 @@ svn_fs_x__id_rev_create(const svn_fs_x__
>
> id->generic_id.vtable = &id_vtable;
> id->generic_id.fsap_data = &id;
> + id->pool = pool;
>
> return (svn_fs_id_t *)id;
> }
> @@ -419,10 +425,10 @@ svn_fs_id_t *
> svn_fs_x__id_copy(const svn_fs_id_t *source, apr_pool_t *pool)
> {
> fs_x__id_t *id = (fs_x__id_t *)source;
> - fs_x__id_t *new_id = apr_palloc(pool, sizeof(*new_id));
> + fs_x__id_t *new_id = apr_pmemdup(pool, id, sizeof(*id));
>
> - *new_id = *id;
> new_id->generic_id.fsap_data = new_id;
> + new_id->pool = pool;
>
> return (svn_fs_id_t *)new_id;
> }
> @@ -444,6 +450,7 @@ svn_fs_x__id_parse(const char *data,
> id = apr_pcalloc(pool, sizeof(*id));
> id->generic_id.vtable = &id_vtable;
> id->generic_id.fsap_data = &id;
> + id->pool = pool;
>
> /* Now, we basically just need to "split" this data on `.'
> characters. We will use svn_cstring_tokenize, which will put
> @@ -531,7 +538,9 @@ svn_fs_x__id_serialize(svn_temp_serializ
> /* Deserialize an ID inside the BUFFER.
> */
> void
> -svn_fs_x__id_deserialize(void *buffer, svn_fs_id_t **in_out)
> +svn_fs_x__id_deserialize(void *buffer,
> + svn_fs_id_t **in_out,
> + apr_pool_t *pool)
> {
> fs_x__id_t *id;
>
> @@ -549,5 +558,6 @@ svn_fs_x__id_deserialize(void *buffer, s
> /* the stored vtable is bogus at best -> set the right one */
> id->generic_id.vtable = &id_vtable;
> id->generic_id.fsap_data = id;
> + id->pool = pool;
> }
>
>
> Modified: subversion/trunk/subversion/libsvn_fs_x/id.h
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/id.
> h?rev=1548823&r1=1548822&r2=1548823&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_fs_x/id.h (original)
> +++ subversion/trunk/subversion/libsvn_fs_x/id.h Sat Dec 7 09:30:42 2013
> @@ -161,11 +161,12 @@ svn_fs_x__id_serialize(struct svn_temp_s
> const svn_fs_id_t * const *id);
>
> /**
> - * Deserialize an @a id within the @a buffer.
> + * Deserialize an @a id within the @a buffer and associate it with @a pool.
> */
> void
> svn_fs_x__id_deserialize(void *buffer,
> - svn_fs_id_t **id);
> + svn_fs_id_t **id,
> + apr_pool_t *pool);
>
> #ifdef __cplusplus
> }
>
> Modified: subversion/trunk/subversion/libsvn_fs_x/temp_serializer.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/te
> mp_serializer.c?rev=1548823&r1=1548822&r2=1548823&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_fs_x/temp_serializer.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_x/temp_serializer.c Sat Dec 7
> 09:30:42 2013
> @@ -324,7 +324,7 @@ deserialize_dir(void *buffer, hash_data_
>
> /* pointer fixup */
> svn_temp_deserializer__resolve(entry, (void **)&entry->name);
> - svn_fs_x__id_deserialize(entry, (svn_fs_id_t **)&entry->id);
> + svn_fs_x__id_deserialize(entry, (svn_fs_id_t **)&entry->id, pool);
>
> /* add the entry to the hash */
> svn_hash_sets(result, entry->name, entry);
> @@ -364,7 +364,8 @@ svn_fs_x__noderev_serialize(svn_temp_ser
>
> void
> svn_fs_x__noderev_deserialize(void *buffer,
> - node_revision_t **noderev_p)
> + node_revision_t **noderev_p,
> + apr_pool_t *pool)
> {
> node_revision_t *noderev;
>
> @@ -378,8 +379,10 @@ svn_fs_x__noderev_deserialize(void *buff
> return;
>
> /* fixup of sub-structures */
> - svn_fs_x__id_deserialize(noderev, (svn_fs_id_t **)&noderev->id);
> - svn_fs_x__id_deserialize(noderev, (svn_fs_id_t **)&noderev-
> >predecessor_id);
> + svn_fs_x__id_deserialize(noderev, (svn_fs_id_t **)&noderev->id, pool);
> + svn_fs_x__id_deserialize(noderev,
> + (svn_fs_id_t **)&noderev->predecessor_id,
> + pool);
> svn_temp_deserializer__resolve(noderev, (void **)&noderev-
> >prop_rep);
> svn_temp_deserializer__resolve(noderev, (void **)&noderev->data_rep);
>
> @@ -687,7 +690,7 @@ svn_fs_x__deserialize_id(void **out,
> svn_fs_id_t *id = (svn_fs_id_t *)data;
>
> /* fixup of all pointers etc. */
> - svn_fs_x__id_deserialize(id, &id);
> + svn_fs_x__id_deserialize(id, &id, pool);
>
> /* done */
> *out = id;
> @@ -733,7 +736,7 @@ svn_fs_x__deserialize_node_revision(void
> node_revision_t *noderev = (node_revision_t *)buffer;
>
> /* fixup of all pointers etc. */
> - svn_fs_x__noderev_deserialize(noderev, &noderev);
> + svn_fs_x__noderev_deserialize(noderev, &noderev, pool);
>
> /* done */
> *item = noderev;
> @@ -891,7 +894,8 @@ svn_fs_x__extract_dir_entry(void **out,
> memcpy(new_entry, source, size);
>
> svn_temp_deserializer__resolve(new_entry, (void **)&new_entry-
> >name);
> - svn_fs_x__id_deserialize(new_entry, (svn_fs_id_t **)&new_entry->id);
> + svn_fs_x__id_deserialize(new_entry, (svn_fs_id_t **)&new_entry->id,
> + pool);
> *(svn_fs_dirent_t **)out = new_entry;
> }
>
> @@ -1085,7 +1089,9 @@ serialize_change(svn_temp_serializer__co
> * serialization CONTEXT.
> */
> static void
> -deserialize_change(void *buffer, change_t **change_p)
> +deserialize_change(void *buffer,
> + change_t **change_p,
> + apr_pool_t *pool)
> {
> change_t * change;
>
> @@ -1097,7 +1103,9 @@ deserialize_change(void *buffer, change_
> return;
>
> /* fix-up of sub-structures */
> - svn_fs_x__id_deserialize(change, (svn_fs_id_t **)&change-
> >info.node_rev_id);
> + svn_fs_x__id_deserialize(change,
> + (svn_fs_id_t **)&change->info.node_rev_id,
> + pool);
>
> svn_temp_deserializer__resolve(change, (void **)&change->path.data);
> svn_temp_deserializer__resolve(change, (void **)&change-
> >info.copyfrom_path);
> @@ -1177,7 +1185,8 @@ svn_fs_x__deserialize_changes(void **out
> for (i = 0; i < changes->count; ++i)
> {
> deserialize_change((void*)changes->changes,
> - (change_t **)&changes->changes[i]);
> + (change_t **)&changes->changes[i],
> + pool);
> APR_ARRAY_PUSH(array, change_t *) = changes->changes[i];
> }
>
>
> Modified: subversion/trunk/subversion/libsvn_fs_x/temp_serializer.h
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/te
> mp_serializer.h?rev=1548823&r1=1548822&r2=1548823&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_fs_x/temp_serializer.h (original)
> +++ subversion/trunk/subversion/libsvn_fs_x/temp_serializer.h Sat Dec 7
> 09:30:42 2013
> @@ -43,11 +43,13 @@ svn_fs_x__noderev_serialize(struct svn_t
> node_revision_t * const *noderev_p);
>
> /**
> - * Deserialize a @a noderev_p within the @a buffer.
> + * Deserialize a @a noderev_p within the @a buffer and associate it with
> + * @a pool.
> */
> void
> svn_fs_x__noderev_deserialize(void *buffer,
> - node_revision_t **noderev_p);
> + node_revision_t **noderev_p,
> + apr_pool_t *pool);
>
> /**
> * Serialize APR array @a *a within the serialization @a context.
>