On 9/20/2018 11:35 AM, Ramsay Jones wrote:
On 20/09/18 00:38, Derrick Stolee wrote:
On 9/18/2018 8:15 PM, Ramsay Jones wrote:
Signed-off-by: Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx>
---
commit-reach.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/commit-reach.h b/commit-reach.h
index 7d313e2975..f41d8f6ba3 100644
--- a/commit-reach.h
+++ b/commit-reach.h
@@ -1,12 +1,13 @@
#ifndef __COMMIT_REACH_H__
#define __COMMIT_REACH_H__
+#include "commit.h"
#include "commit-slab.h"
-struct commit;
struct commit_list;
-struct contains_cache;
Interesting that you needed all of commit.h here, and these 'struct commit' and 'struct contains_cache' were not enough. Was there a reason you needed the entire header file instead of just adding a missing struct declaration?
Actually, the original version of this patch didn't add that
include! I changed my mind just before sending this series
out, since I felt the original patch was conflating two issues.
The reason for the #include of 'commit.h' in this patch, but
not with my original patch, has to to with the inline functions
used in the commit-slab implementation. My original patch used
the 'commit-slab-{decl,impl}.h' header files to sidestep the
need for the definition of 'struct commit'.
I have included an 'RFC on-top' patch below to show you what I
had in mind. NOTE: I had not finished writing the commit message
and you could argue that the 'implement' macro should go in the
ref-filter.c file, since that is were the contains_cache is
'defined and initialised'. I had not intended to send this to
the list ... :-D
Note that, if you compile with optimizations disabled, then
run this script:
$ cat -n dup-static.sh
1 #!/bin/sh
2
3 nm $1 | grep ' t ' | cut -d' ' -f3 | sort | uniq -c |
4 sort -rn | grep -v ' 1'
$
$ ./dup-static.sh git | grep contains
24 init_contains_cache_with_stride
24 init_contains_cache
24 contains_cache_peek
24 contains_cache_at_peek
24 contains_cache_at
24 clear_contains_cache
$
you will find 24 copies of the commit-slab routines for the
contains_cache. Of course, when you enable optimizations again,
these duplicate static functions (mostly) disappear. (Two static
functions remain, the rest are actually inlined at -O2).
However, if you apply the patch below, you end up with all of
the functions in the contains_cache commit-slab implementation
as external functions. Some of those functions are never called,
so it's not necessarily a win ... ;-)
Thanks for the explanation! I prefer your #include "commit.h" above to
the alternative.
Thanks,
-Stolee
