You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ko...@apache.org on 2016/01/09 19:58:48 UTC
svn commit: r1723876 - in /subversion/branches/fs-node-api/subversion:
include/svn_fs.h libsvn_fs/fs-loader.c libsvn_fs/fs-loader.h
libsvn_fs/node_compat.c libsvn_fs_fs/node.c
Author: kotkov
Date: Sat Jan 9 18:58:48 2016
New Revision: 1723876
URL: http://svn.apache.org/viewvc?rev=1723876&view=rev
Log:
On 'fs-node-api' branch: Allow getting the filesystem to which a particular
filesystem node belongs. This is a prerequisite for switching the remaining
parts of svn_ra_local__get_dir() to the new FS node API.
* subversion/include/svn_fs.h
(svn_fs_node_fs): New function.
* subversion/libsvn_fs/fs-loader.h
(struct svn_fs_node_t): Add 'fs' member to this structure.
* subversion/libsvn_fs/fs-loader.c
(svn_fs_node_fs): Implement this new function.
* subversion/libsvn_fs/node_compat.c
(svn_fs__create_node_shim): Fill in node->fs member.
* subversion/libsvn_fs_fs/node.c
(svn_fs_fs__node_create): Fill in node->fs member.
Modified:
subversion/branches/fs-node-api/subversion/include/svn_fs.h
subversion/branches/fs-node-api/subversion/libsvn_fs/fs-loader.c
subversion/branches/fs-node-api/subversion/libsvn_fs/fs-loader.h
subversion/branches/fs-node-api/subversion/libsvn_fs/node_compat.c
subversion/branches/fs-node-api/subversion/libsvn_fs_fs/node.c
Modified: subversion/branches/fs-node-api/subversion/include/svn_fs.h
URL: http://svn.apache.org/viewvc/subversion/branches/fs-node-api/subversion/include/svn_fs.h?rev=1723876&r1=1723875&r2=1723876&view=diff
==============================================================================
--- subversion/branches/fs-node-api/subversion/include/svn_fs.h (original)
+++ subversion/branches/fs-node-api/subversion/include/svn_fs.h Sat Jan 9 18:58:48 2016
@@ -1633,6 +1633,10 @@ svn_fs_paths_changed(apr_hash_t **change
*/
typedef struct svn_fs_node_t svn_fs_node_t;
+/** Return the filesystem to which @a node belongs. */
+svn_fs_t *
+svn_fs_node_fs(svn_fs_node_t *node);
+
/** Open the node present at @a path under @a root. Set @a *node_p to the node
* Sets @a *node_p to NULL if @a path does not exist under @a root
* and @a ignore_enoent is non-zero. Returns error otherwise.
Modified: subversion/branches/fs-node-api/subversion/libsvn_fs/fs-loader.c
URL: http://svn.apache.org/viewvc/subversion/branches/fs-node-api/subversion/libsvn_fs/fs-loader.c?rev=1723876&r1=1723875&r2=1723876&view=diff
==============================================================================
--- subversion/branches/fs-node-api/subversion/libsvn_fs/fs-loader.c (original)
+++ subversion/branches/fs-node-api/subversion/libsvn_fs/fs-loader.c Sat Jan 9 18:58:48 2016
@@ -1054,6 +1054,12 @@ svn_fs_check_path(svn_node_kind_t *kind_
return svn_error_trace(root->vtable->check_path(kind_p, root, path, pool));
}
+svn_fs_t *
+svn_fs_node_fs(svn_fs_node_t *node)
+{
+ return node->fs;
+}
+
svn_error_t *
svn_fs_open_node(svn_fs_node_t **node_p,
svn_fs_root_t *root,
Modified: subversion/branches/fs-node-api/subversion/libsvn_fs/fs-loader.h
URL: http://svn.apache.org/viewvc/subversion/branches/fs-node-api/subversion/libsvn_fs/fs-loader.h?rev=1723876&r1=1723875&r2=1723876&view=diff
==============================================================================
--- subversion/branches/fs-node-api/subversion/libsvn_fs/fs-loader.h (original)
+++ subversion/branches/fs-node-api/subversion/libsvn_fs/fs-loader.h Sat Jan 9 18:58:48 2016
@@ -591,6 +591,9 @@ struct svn_fs_lock_target_t
struct svn_fs_node_t
{
+ /* The filesystem to which this node belongs */
+ svn_fs_t *fs;
+
/* FSAP-specific vtable and private data */
const node_vtable_t *vtable;
void *fsap_data;
Modified: subversion/branches/fs-node-api/subversion/libsvn_fs/node_compat.c
URL: http://svn.apache.org/viewvc/subversion/branches/fs-node-api/subversion/libsvn_fs/node_compat.c?rev=1723876&r1=1723875&r2=1723876&view=diff
==============================================================================
--- subversion/branches/fs-node-api/subversion/libsvn_fs/node_compat.c (original)
+++ subversion/branches/fs-node-api/subversion/libsvn_fs/node_compat.c Sat Jan 9 18:58:48 2016
@@ -164,6 +164,7 @@ svn_fs__create_node_shim(svn_fs_root_t *
fnd->path = apr_pstrdup(result_pool, path);
fnd->node_kind = kind;
+ node->fs = svn_fs_root_fs(root);
node->vtable = &compat_node_vtable;
node->fsap_data = fnd;
Modified: subversion/branches/fs-node-api/subversion/libsvn_fs_fs/node.c
URL: http://svn.apache.org/viewvc/subversion/branches/fs-node-api/subversion/libsvn_fs_fs/node.c?rev=1723876&r1=1723875&r2=1723876&view=diff
==============================================================================
--- subversion/branches/fs-node-api/subversion/libsvn_fs_fs/node.c (original)
+++ subversion/branches/fs-node-api/subversion/libsvn_fs_fs/node.c Sat Jan 9 18:58:48 2016
@@ -112,6 +112,7 @@ svn_fs_fs__node_create(dag_node_t *dag_n
fs_node_data_t *fnd = apr_palloc(result_pool, sizeof(*fnd));
svn_fs_node_t *node = apr_palloc(result_pool, sizeof(*node));
fnd->dag_node = dag_node;
+ node->fs = svn_fs_fs__dag_get_fs(dag_node);
node->vtable = &fs_node_vtable;
node->fsap_data = fnd;
Re: svn commit: r1723876 - in /subversion/branches/fs-node-api/subversion:
include/svn_fs.h libsvn_fs/fs-loader.c libsvn_fs/fs-loader.h
libsvn_fs/node_compat.c libsvn_fs_fs/node.c
Posted by Evgeny Kotkov <ev...@visualsvn.com>.
Bert Huijben <be...@qqmail.nl> writes:
> The more interesting question is: what is the status of this branch?
>
> I would like to see this merged to trunk, unless there are some new problems.
(Sorry for the delay in response.)
I think that the new API makes sense by itself, and could be useful. But
we didn't see any noticeable performance improvement from switching to it.
Although the server does less work, the actual numbers are only less by a
few percent.
I plan to switch the last remaining callers in reporter.c to the new API, and
then we can decide on what to do next.
> I would say the relation is +-:
>
> Fs -> root -> node.
>
> In that case I would say the node should hold a pointer to root (which
> already holds a pointer to fs).
>
> Why add a direct pointer 2 levels up, when the step via root is more
> obvious and probably already available.
>
> (This is all about the member variable... I don't see a problem with the
> accessor function. It might not be needed later on after more cleanup as
> callers most likely already have all/most these pointers itself)
A node is detached from the particular root. The reason behind this is that
a root can be explicitly deallocated with svn_fs_close_root() (that's what
the reporter does when it keeps a fixed size LRU cache for open roots),
and this approach avoids having a node possibly referring to a deallocated
svn_fs_root_t.
As for the member variable, I think that we can replace it with a vtable
accessor, since we do have the information about svn_fs_t on lower layers.
Regards,
Evgeny Kotkov
RE: svn commit: r1723876 - in /subversion/branches/fs-node-api/subversion: include/svn_fs.h libsvn_fs/fs-loader.c libsvn_fs/fs-loader.h libsvn_fs/node_compat.c libsvn_fs_fs/node.c
Posted by Bert Huijben <be...@qqmail.nl>.
See inline review below of a minor issue I found when this was committed, but didn't mail about then.
The more interesting question is: what is the status of this branch?
I would like to see this merged to trunk, unless there are some new problems.
> -----Original Message-----
> From: kotkov@apache.org [mailto:kotkov@apache.org]
> Sent: zaterdag 9 januari 2016 19:59
> To: commits@subversion.apache.org
> Subject: svn commit: r1723876 - in /subversion/branches/fs-node-
> api/subversion: include/svn_fs.h libsvn_fs/fs-loader.c libsvn_fs/fs-loader.h
> libsvn_fs/node_compat.c libsvn_fs_fs/node.c
>
> Author: kotkov
> Date: Sat Jan 9 18:58:48 2016
> New Revision: 1723876
>
> URL: http://svn.apache.org/viewvc?rev=1723876&view=rev
> Log:
> On 'fs-node-api' branch: Allow getting the filesystem to which a particular
> filesystem node belongs. This is a prerequisite for switching the remaining
> parts of svn_ra_local__get_dir() to the new FS node API.
>
> * subversion/include/svn_fs.h
> (svn_fs_node_fs): New function.
>
> * subversion/libsvn_fs/fs-loader.h
> (struct svn_fs_node_t): Add 'fs' member to this structure.
I would say the relation is +-:
Fs -> root -> node.
In that case I would say the node should hold a pointer to root (which already holds a pointer to fs).
Why add a direct pointer 2 levels up, when the step via root is more obvious and probably already available.
(This is all about the member variable... I don't see a problem with the accessor function. It might not be needed later on after more cleanup as callers most likely already have all/most these pointers itself)
Bert