Re: [PATCH 15/17] commit-slab: provide a static initializer

2014-06-12 Thread Jeff King
On Thu, Jun 12, 2014 at 11:15:49AM -0700, Junio C Hamano wrote:

> Why do we need an initialiser at this point (in other words, how
> have other existing slab users coped without having one)?
> 
> I think they call init_*_slab() when the slab is needed/used the
> first time (e.g. it is not even worth initialising indegree slab
> unless we are sorting in topo order).
> 
> Unlike the author-date and indegree slabs, there are too many entry
> points that may want access to the buffer slab (save_commit_buffer's
> default being on does not help us either), so it would be much less
> error prone to always initialise a static slab like this patch does,
> I guess.

Yes, and also because the lifetime of this slab is different. For the
indegree and author-date slabs, the slab lives for one operation.  So we
init the slab at the beginning of the operation, do the sort, then clear
it at the end.

This slab does not exist for one operation. It is a global cache that
can be used by any code, just like the "struct commit" cache itself.. It
should always be initialized from the start, and continue until the
program exits.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 15/17] commit-slab: provide a static initializer

2014-06-12 Thread Junio C Hamano
Jeff King  writes:

> Callers currently must use init_foo_slab() at runtime before
> accessing a slab. For global slabs, it's much nicer if we
> can initialize them in BSS, so that each user does not have
> to add code to check-and-initialize.
>
> Signed-off-by: Jeff King 
> ---
> There was no comment on this one in v1. I'd be curious if anyone has
> comments on what I wrote in:
>
>   http://article.gmane.org/gmane.comp.version-control.git/251099

Why do we need an initialiser at this point (in other words, how
have other existing slab users coped without having one)?

I think they call init_*_slab() when the slab is needed/used the
first time (e.g. it is not even worth initialising indegree slab
unless we are sorting in topo order).

Unlike the author-date and indegree slabs, there are too many entry
points that may want access to the buffer slab (save_commit_buffer's
default being on does not help us either), so it would be much less
error prone to always initialise a static slab like this patch does,
I guess.


>  commit-slab.h | 12 
>  1 file changed, 12 insertions(+)
>
> diff --git a/commit-slab.h b/commit-slab.h
> index cc114b5..375c9c7 100644
> --- a/commit-slab.h
> +++ b/commit-slab.h
> @@ -117,4 +117,16 @@ static int stat_ ##slabname## realloc
>   * catch because GCC silently parses it by default.
>   */
>  
> +/*
> + * Statically initialize a commit slab named "var". Note that this
> + * evaluates "stride" multiple times! Example:
> + *
> + *   struct indegree indegrees = COMMIT_SLAB_INIT(1, indegrees);
> + *
> + */
> +#define COMMIT_SLAB_INIT(stride, var) { \
> + COMMIT_SLAB_SIZE / sizeof(**((var).slab)) / (stride), \
> + (stride), 0, NULL \
> +}
> +
>  #endif /* COMMIT_SLAB_H */
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 15/17] commit-slab: provide a static initializer

2014-06-10 Thread Jeff King
Callers currently must use init_foo_slab() at runtime before
accessing a slab. For global slabs, it's much nicer if we
can initialize them in BSS, so that each user does not have
to add code to check-and-initialize.

Signed-off-by: Jeff King 
---
There was no comment on this one in v1. I'd be curious if anyone has
comments on what I wrote in:

  http://article.gmane.org/gmane.comp.version-control.git/251099

 commit-slab.h | 12 
 1 file changed, 12 insertions(+)

diff --git a/commit-slab.h b/commit-slab.h
index cc114b5..375c9c7 100644
--- a/commit-slab.h
+++ b/commit-slab.h
@@ -117,4 +117,16 @@ static int stat_ ##slabname## realloc
  * catch because GCC silently parses it by default.
  */
 
+/*
+ * Statically initialize a commit slab named "var". Note that this
+ * evaluates "stride" multiple times! Example:
+ *
+ *   struct indegree indegrees = COMMIT_SLAB_INIT(1, indegrees);
+ *
+ */
+#define COMMIT_SLAB_INIT(stride, var) { \
+   COMMIT_SLAB_SIZE / sizeof(**((var).slab)) / (stride), \
+   (stride), 0, NULL \
+}
+
 #endif /* COMMIT_SLAB_H */
-- 
2.0.0.729.g520999f

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html