Re: [PATCH] hashmap: add API to disable item counting when threaded
From: Junio C Hamano <hidden>
Date: 2017-09-06 01:24:47
Jeff Hostetler [off-list ref] writes:
From: Jeff Hostetler <redacted> This is to address concerns raised by ThreadSanitizer on the mailing list about threaded unprotected R/W access to map.size with my previous "disallow rehash" change (0607e10009ee4e37cb49b4cec8d28a9dda1656a4). See: https://public-inbox.org/git/adb37b70139fd1e2bac18bfd22c8b96683ae18eb.1502780344.git.martin.agren@gmail.com/ Add API to hashmap to disable item counting and to disable automatic rehashing. Also include APIs to re-enable item counting and automatica rehashing. When item counting is disabled, the map.size field is invalid. So to prevent accidents, the field has been renamed and an accessor function hashmap_get_size() has been added. All direct references to this field have been been updated. And the name of the field changed to map.private_size to communicate thie. Signed-off-by: Jeff Hostetler <redacted> --- attr.c | 14 ++++--- builtin/describe.c | 2 +- hashmap.c | 31 +++++++++++----- hashmap.h | 98 ++++++++++++++++++++++++++++++++++++++----------- name-hash.c | 6 ++- t/helper/test-hashmap.c | 2 +- 6 files changed, 113 insertions(+), 40 deletions(-)
I feel somewhat stupid to say this, especially after seeing many
people applaud this patch, but I do not seem to be able to even
build Git with this patch. I am getting:
common-main.o: In function `hashmap_get_size':
/home/gitster/w/git.git/hashmap.h:260: multiple definition of `hashmap_get_size'
fast-import.o:/home/gitster/w/git.git/hashmap.h:260: first defined here
libgit.a(argv-array.o): In function `hashmap_get_size':
/home/gitster/w/git.git/hashmap.h:260: multiple definition of `hashmap_get_size'
fast-import.o:/home/gitster/w/git.git/hashmap.h:260: first defined here
libgit.a(attr.o): In function `hashmap_get_size':
...
and wonder if others are building with different options or something..
quoted hunk ↗ jump to hunk
diff --git a/hashmap.h b/hashmap.h index 7a8fa7f..7b8e6f4 100644 --- a/hashmap.h +++ b/hashmap.h@@ -253,6 +253,19 @@ static inline void hashmap_entry_init(void *entry, unsigned int hash) } /* + * Return the number of items in the map. + */ +inline unsigned int hashmap_get_size(struct hashmap *map)
I think this must become static inline like everybody else in this file, at least. I also wonder if this header is excessively inlining too many functions without measuring first, but that is a different story.