Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Am 09.12.2013 15:03, schrieb Karsten Blees: > 3.) Inject individual fields via macro > > Instead of injecting a struct hashmap_entry, which implies alignment to > sizeof(struct hashmap_entry), we could inject the individual fields, e.g. > > #define HASHMAP_ENTRY_HEADER struct hashmap_entry __next; unsinged int > __hash; > > struct name_entry { >HASHMAP_ENTRY_HEADER >int namelen; >char *name; > }; > I've tried this as well. However, the change is much more intrusive, and produces lots of strict-aliasing warnings in GCC 4.4, probably due to this bug [1]. So I don't think its a good solution. For anyone interested, the patch can be found at [2]. [1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42032 [2] https://github.com/kblees/git/commits/kb/hashmap-v5-fixes-macro -- 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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Am 18.12.2013 14:10, schrieb Karsten Blees: > + printf("sizeof(pointer+int) (%u) is not a " > +"multiple of sizeof(pointer) (%u)!\n", > +sizeof(struct pointer_int), > +sizeof(void *)); Should be: - sizeof(struct pointer_int), - sizeof(void *)); + (unsigned) sizeof(struct pointer_int), + (unsigned) sizeof(void *)); (sizeof() is 'unsigned int' on 32-bit and 'long unsigned int' on 64-bit; without the cast, "%u" produces warnings on 64-bit and "%lu" on 32-bit...) -- 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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Am 10.12.2013 00:45, schrieb Jonathan Nieder: > Karsten Blees wrote: > >> Googling some more, I believe the most protable way to achieve this >> via 'compiler settings' is >> >> #pragma pack(push) >> #pragma pack(4) >> struct hashmap_entry { >>struct hashmap_entry *next; >>unsigned int hash; >> }; >> #pragma pack(pop) >> >> This is supported by at least GCC, MSVC and HP (see your link). The >> downside is that we cannot use macros (in git-compat-util.h) to emit >> #pragmas. But we wouldn't have to, as compilers aren't supposed to >> barf on unknown #pragmas. > > Technically this can be done using macros: > > #if (gcc) > #define BEGIN_PACKED _Pragma("pack(push,4)") > #define END_PACKED _Pragma("pack(pop)") > #elif (msvc) > #define BEGIN_PACKED __pragma(pack(push,4)) > #define END_PACKED __pragma(pack(pop)) > #else > /* Just tolerate a little padding. */ > #define BEGIN_PACKED > #define END_PACKED > #end > > Then you can do: > > BEGIN_PACKED > struct hashmap_entry { > ... > }; > END_PACKED > Sorry for the delay... My intention with #pragma pack was to support as many compilers / platforms as possible (even though I can't test them all). From what I could find on the 'net, support for the _Pragma operator is slim. And as the MSVC build is 32-bit only (AFAIK), this is pretty much a GCC-only solution (and thus equivalent, but much more verbose than the initial __attribute__((__packed__)) variant). > Whether that's nicer or uglier than the alternatives I leave to you. > ;-) If it was really up to me ( :-) ), I'd like to do this: -- 8< --- Subject: [PATCH] hashmap.h: make sure map entries are tightly packed Extending 'struct hashmap_entry' with an int-sized member shouldn't waste memory on 64-bit systems. This is already documented in api-hashmap.txt, but struct hashmap_entry needs to be packed for this to work. E.g. struct name_entry { struct hashmap_entry ent; /* consists of pointer + int */ int namelen; char *name; }; should be 24 instead of 32 bytes. Packing structures is compiler-specific, most compilers support [1]: #pragma pack(n) - to set structure packing #pragma pack() - to reset structure packing to default Compilers are supposed to ignore unknown #pragmas, so using this without further #if guards should be safe. Wasting a few bytes for padding is acceptable, however, compiling the rest of git with packed structures (and thus mis-aligned arrays of structs) is not. Add a test to ensure that '#pragma pack()' really resets the packing. [1] Compiler docs regarding #pragma pack: http://gcc.gnu.org/onlinedocs/gcc-4.6.2/gcc/Structure_002dPacking-Pragmas.html#Structure_002dPacking-Pragmas http://msdn.microsoft.com/en-us/library/2e70t5y1%28v=vs.80%29.aspx http://h21007.www2.hp.com/portal/download/files/unprot/aCxx/Online_Help/pragmas.htm#pragma-PACK http://clang.llvm.org/docs/UsersManual.html#microsoft-extensions http://uw714doc.sco.com/en/SDK_cprog/_Preprocessing_Directives.html http://osr507doc.sco.com/en/tools/ANSI_F.3.13_Preprocessing.html http://docs.oracle.com/cd/E19422-01/819-3690/Pragmas_App.html#73499 http://publib.boulder.ibm.com/infocenter/comphelp/v7v91/index.jsp?topic=%2Fcom.ibm.vacpp7a.doc%2Fcompiler%2Fref%2Frnpgpack.htm Supposedly wants #pragma pack(0) to reset: http://techpubs.sgi.com/library/tpl/cgi-bin/getdoc.cgi/0650/bks/SGI_Developer/books/Pragmas/sgi_html/ch04.html Not supported: http://software.intel.com/sites/products/documentation/doclib/iss/2013/compiler/cpp-lin/GUID-DD32852C-A0F9-4AC6-BF67-D10D064CC87A.htm Signed-off-by: Karsten Blees --- hashmap.h | 7 +++ t/t0011-hashmap.sh | 6 ++ test-hashmap.c | 17 + 3 files changed, 30 insertions(+) diff --git a/hashmap.h b/hashmap.h index a816ad4..93b330b 100644 --- a/hashmap.h +++ b/hashmap.h @@ -15,10 +15,17 @@ extern unsigned int memihash(const void *buf, size_t len); /* data structures */ +/* + * Set struct packing to 4 so that we don't waste memory on 64-bit systems. + * Struct hashmap_entry is used as prefix to other (un-packed) structures, + * so this won't cause alignment issues e.g. for odd elements in an array. + */ +#pragma pack(4) struct hashmap_entry { struct hashmap_entry *next; unsigned int hash; }; +#pragma pack() typedef int (*hashmap_cmp_fn)(const void *entry, const void *entry_or_key, const void *keydata); diff --git a/t/t0011-hashmap.sh b/t/t0011-hashmap.sh index 391e2b6..3f3c90b 100755 --- a/t/t0011-hashmap.sh +++ b/t/t0011-hashmap.sh @@ -237,4 +237,10 @@ test_expect_success 'grow / shrink' ' ' +test_expect_success '"#pragma pack()" resets packing to default' ' + + test_hashmap "pragma-pack" "ok" + +' + test_done diff --git a/test-hashmap.c b/test-hashmap.c index f5183fb..2d5a63a 100644 --- a/test-hashmap.c +++ b/test-hashmap.c @@ -36,6 +36,12 @@ static struct test_entry *alloc_test_entry(int hash, char *key, int klen,
Re: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Karsten Blees wrote: > (Besides, __attribute__((aligned)) / __declspec(align) can only > _increase_ the alignment, so aligned(1) would have no effect). Good catch. > Googling some more, I believe the most protable way to achieve this > via 'compiler settings' is > > #pragma pack(push) > #pragma pack(4) > struct hashmap_entry { >struct hashmap_entry *next; >unsigned int hash; > }; > #pragma pack(pop) > > This is supported by at least GCC, MSVC and HP (see your link). The > downside is that we cannot use macros (in git-compat-util.h) to emit > #pragmas. But we wouldn't have to, as compilers aren't supposed to > barf on unknown #pragmas. Technically this can be done using macros: #if (gcc) # define BEGIN_PACKED _Pragma("pack(push,4)") # define END_PACKED _Pragma("pack(pop)") #elif (msvc) # define BEGIN_PACKED __pragma(pack(push,4)) # define END_PACKED __pragma(pack(pop)) #else /* Just tolerate a little padding. */ # define BEGIN_PACKED # define END_PACKED #end Then you can do: BEGIN_PACKED struct hashmap_entry { ... }; END_PACKED Whether that's nicer or uglier than the alternatives I leave to you. ;-) Thanks, Jonathan -- 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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Am 09.12.2013 21:08, schrieb Jonathan Nieder: > Karsten Blees wrote: > >> GCC supports __packed__ as of 2.3 (1992), so any other compilers >> that copied the __attribute__ feature probably won't complain. > > Alas, it looks like HP C doesn't support __packed__ (not that I > care much about HP C): > > > http://h21007.www2.hp.com/portal/download/files/unprot/aCxx/Online_Help/pragmas.htm#Attributes > Thanks for the link > Maybe a macro expanding to __attribute__((aligned(1))) on the fields > would work? The same macro could expand to __declspec(align(1)) in > the MSVC build. > But alignment is not the same as packing. We still want the structure to be 8-byte aligned (i.e. variables of the type should start at 8-byte boundaries). We just don't want the size of the structure to be padded to a multiple of 8, so that we can extend it without penalty. (Besides, __attribute__((aligned)) / __declspec(align) can only _increase_ the alignment, so aligned(1) would have no effect). Googling some more, I believe the most protable way to achieve this via 'compiler settings' is #pragma pack(push) #pragma pack(4) struct hashmap_entry { struct hashmap_entry *next; unsigned int hash; }; #pragma pack(pop) This is supported by at least GCC, MSVC and HP (see your link). The downside is that we cannot use macros (in git-compat-util.h) to emit #pragmas. But we wouldn't have to, as compilers aren't supposed to barf on unknown #pragmas. However, considering the portability issues, the macro solution (injecting just the two fields instead of a struct) becomes more and more attractive in my mind... Karsten -- 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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Jeff King writes: > On Sat, Dec 07, 2013 at 06:03:22PM +0100, Thomas Rast wrote: > >> Junio C Hamano writes: >> >> > * jk/pack-bitmap (2013-11-18) 22 commits >> [...] >> Peff can decide if he wants to reroll with my nits or not; either way >> I'm all for moving it forward and aiming for one of the next releases. > > I'm going to be a bit slow this week, as I'm traveling in China. > > I have at least one more local fix queued up (one of the re-rolls > introduced a use-after-free). Your comments make sense to me, though > some of them are "if this is not too hard", and I haven't looked yet to > see how hard some of the requisite refactoring would be. So expect at > least one more re-roll, and I'll try to incorporate your comments. > Thanks for giving it a careful reading. > > As an aside, we have been running the last version sent to the list > (modulo the fix I mentioned above) on github.com for a week or two > (previously we were running the old, based-on-v1.8.4 version). So it is > getting exercised. ;-). -- 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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
On Sat, Dec 07, 2013 at 06:03:22PM +0100, Thomas Rast wrote: > Junio C Hamano writes: > > > * jk/pack-bitmap (2013-11-18) 22 commits > [...] > Peff can decide if he wants to reroll with my nits or not; either way > I'm all for moving it forward and aiming for one of the next releases. I'm going to be a bit slow this week, as I'm traveling in China. I have at least one more local fix queued up (one of the re-rolls introduced a use-after-free). Your comments make sense to me, though some of them are "if this is not too hard", and I haven't looked yet to see how hard some of the requisite refactoring would be. So expect at least one more re-roll, and I'll try to incorporate your comments. Thanks for giving it a careful reading. As an aside, we have been running the last version sent to the list (modulo the fix I mentioned above) on github.com for a week or two (previously we were running the old, based-on-v1.8.4 version). So it is getting exercised. -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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Felipe Contreras writes: > To take hostage the whole patch series because of the name > --refspec (which is perfectly fine) is just an excuse to not fix > the issues already present. I actually was wondering why you are taking the fixes to the hostage to force a wrong option name. -- 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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Karsten Blees wrote: > GCC supports __packed__ as of 2.3 (1992), so any other compilers > that copied the __attribute__ feature probably won't complain. Alas, it looks like HP C doesn't support __packed__ (not that I care much about HP C): http://h21007.www2.hp.com/portal/download/files/unprot/aCxx/Online_Help/pragmas.htm#Attributes Maybe a macro expanding to __attribute__((aligned(1))) on the fields would work? The same macro could expand to __declspec(align(1)) in the MSVC build. Thanks, Jonathan -- 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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Karsten Blees writes: >> * kb/fast-hashmap (2013-11-18) 14 commits >> (merged to 'next' on 2013-12-06 at f90be3d) > > Damn, a day too late :-) I found these two glitches today...is a > fixup patch OK or should I do a reroll (or separate patch on top)? A separate patch on top would be the most appropriate. People have been looking at the change since mid November, and nobody noticed the problem; having a separate fix on top is a good way to document what the specific gotcha that can be easily missed is. I think the patch you attached describes the issue well, possibly with a retitle (perhaps "hashmap.h: make sure map entries are tightly packed", or something.) Thanks. > --- 8< --- > Subject: [PATCH] fixup! add a hashtable implementation that supports O(1) > removal > > Use 'unsigned int' for hash-codes everywhere. > > Extending 'struct hashmap_entry' with an int-sized member shouldn't waste > memory on 64-bit systems. This is already documented in api-hashmap.txt, > but needs '__attribute__((__packed__))' to work. Reduces e.g. > > struct name_entry { > struct hashmap_entry ent; > int namelen; > char *name; > }; > > from 32 to 24 bytes. > > Signed-off-by: Karsten Blees > --- > hashmap.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hashmap.h b/hashmap.h > index f5b3b61..b64567b 100644 > --- a/hashmap.h > +++ b/hashmap.h > @@ -15,7 +15,7 @@ extern unsigned int memihash(const void *buf, size_t len); > > /* data structures */ > > -struct hashmap_entry { > +struct __attribute__((__packed__)) hashmap_entry { > struct hashmap_entry *next; > unsigned int hash; > }; > @@ -43,7 +43,7 @@ extern void hashmap_free(struct hashmap *map, int > free_entries); > > /* hashmap_entry functions */ > > -static inline void hashmap_entry_init(void *entry, int hash) > +static inline void hashmap_entry_init(void *entry, unsigned int hash) > { > struct hashmap_entry *e = entry; > e->hash = hash; -- 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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Karsten Blees writes: > Am 07.12.2013 00:52, schrieb Junio C Hamano: > >> * kb/doc-exclude-directory-semantics (2013-11-07) 1 commit >> - gitignore.txt: clarify recursive nature of excluded directories >> >> Originally merged to 'next' on 2013-11-13 >> >> Kicked back to 'pu' to replace with a newer reroll ($gmane/237814 >> looked OK but there seems to have some loose ends in the >> discussion). > > I'm unaware of any loose ends, could you clarify? > > Btw. $gmane/237814 seems to be a different topic, the version in next (and > now in pu) was $gmane/237429. Ah, my mistake. I thought I still had $gmane/237416 and needed to wait for the discussion to conclude; in fact, a later one in that thread $gmane/237429 is what came out of that discussion, so this is good to go, perhaps with an amend with Acked-by: peff. Thanks. -- 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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Am 08.12.2013 11:20, schrieb Thomas Rast: > Karsten Blees writes: > >> Am 07.12.2013 23:23, schrieb Thomas Rast: >>> Karsten Blees writes: >>> Extending 'struct hashmap_entry' with an int-sized member shouldn't waste memory on 64-bit systems. This is already documented in api-hashmap.txt, but needs '__attribute__((__packed__))' to work. Reduces e.g. >>> >>> You'd have to guard __attribute__((__packed__)) with some compiler >>> detection in git-compat-util.h though. >>> >> >> Isn't that already handled? __attribute__ is already widely used >> (e.g. for printf formats), and platforms that don't support it define >> it as empty (e.g. MSVC). Or do you mean I should account for >> compiler-specific variants (#pragma pack...)? > > True, __attribute__ expands to nothing on unknown compilers, but what > does the compiler do when it sees an unknown attribute? If some of them > choke, you need a separate macro. > > I'm a bit confused myself though, many attributes have special macros in > git-compat-util.h but others we just use in the code. > So what do you propose? I basically see three options: 1.) Trial and error GCC supports __packed__ as of 2.3 (1992), so any other compilers that copied the __attribute__ feature probably won't complain. 2.) Accept the wasted memory Moving the hash code from the hash table (as in hash.[ch]) to the entry is already a big improvement, as it no longer multiplies hash code + wasted bytes with the load factor. 64-bit software uses more memory in general, so we could just live with it (and only fix the documentation in api-hashmap.txt). 3.) Inject individual fields via macro Instead of injecting a struct hashmap_entry, which implies alignment to sizeof(struct hashmap_entry), we could inject the individual fields, e.g. #define HASHMAP_ENTRY_HEADER struct hashmap_entry __next; unsinged int __hash; struct name_entry { HASHMAP_ENTRY_HEADER int namelen; char *name; }; What do you think? Karsten -- 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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Karsten Blees writes: > Am 07.12.2013 23:23, schrieb Thomas Rast: >> Karsten Blees writes: >> >>> Extending 'struct hashmap_entry' with an int-sized member shouldn't waste >>> memory on 64-bit systems. This is already documented in api-hashmap.txt, >>> but needs '__attribute__((__packed__))' to work. Reduces e.g. >> >> You'd have to guard __attribute__((__packed__)) with some compiler >> detection in git-compat-util.h though. >> > > Isn't that already handled? __attribute__ is already widely used > (e.g. for printf formats), and platforms that don't support it define > it as empty (e.g. MSVC). Or do you mean I should account for > compiler-specific variants (#pragma pack...)? True, __attribute__ expands to nothing on unknown compilers, but what does the compiler do when it sees an unknown attribute? If some of them choke, you need a separate macro. I'm a bit confused myself though, many attributes have special macros in git-compat-util.h but others we just use in the code. -- Thomas Rast t...@thomasrast.ch -- 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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Am 07.12.2013 23:23, schrieb Thomas Rast: > Karsten Blees writes: > >> Extending 'struct hashmap_entry' with an int-sized member shouldn't waste >> memory on 64-bit systems. This is already documented in api-hashmap.txt, >> but needs '__attribute__((__packed__))' to work. Reduces e.g. > > You'd have to guard __attribute__((__packed__)) with some compiler > detection in git-compat-util.h though. > Isn't that already handled? __attribute__ is already widely used (e.g. for printf formats), and platforms that don't support it define it as empty (e.g. MSVC). Or do you mean I should account for compiler-specific variants (#pragma pack...)? -- 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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Karsten Blees writes: > Extending 'struct hashmap_entry' with an int-sized member shouldn't waste > memory on 64-bit systems. This is already documented in api-hashmap.txt, > but needs '__attribute__((__packed__))' to work. Reduces e.g. You'd have to guard __attribute__((__packed__)) with some compiler detection in git-compat-util.h though. -- Thomas Rast t...@thomasrast.ch -- 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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
On Sat, Dec 7, 2013 at 4:03 AM, Felipe Contreras wrote: > Junio C Hamano wrote: >> * fc/transport-helper-fixes (2013-11-13) 12 commits >> - remote-bzr: support the new 'force' option >> - transport-helper: add support to delete branches >> - fast-export: add support to delete refs >> - fast-import: add support to delete refs >> - transport-helper: add support for old:new refspec >> - fast-export: add new --refspec option >> - fast-export: improve argument parsing >> - test-hg.sh: tests are now expected to pass >> - transport-helper: check for 'forced update' message >> - transport-helper: add 'force' to 'export' helpers >> - transport-helper: don't update refs in dry-run >> - transport-helper: mismerge fix >> >> Updates transport-helper, fast-import and fast-export to allow the >> ref mapping and ref deletion in a way similar to the natively >> supported transports. >> >> The option name "--refspec" needs to be rethought. It does not mean >> what refspec usually means, even though it shares the same syntax >> with refspec; calling it --refspec only because it shares the same >> syntax is like calling it --asciistring and does not make sense. > > Not true. remote..fetch is a refspec, and doesn't specify what to fetch, > so does the "refspec" capability in remote helpers, and so does the proposed > --refspec option. > > It is exactly what it already means. Not to mention that you can apply the first part of the series without any problems: >> - fast-export: improve argument parsing >> - test-hg.sh: tests are now expected to pass >> - transport-helper: check for 'forced update' message >> - transport-helper: add 'force' to 'export' helpers >> - transport-helper: don't update refs in dry-run >> - transport-helper: mismerge fix Even: >> - remote-bzr: support the new 'force' option The rest is for old:new :to-delete refspecs. To take hostage the whole patch series because of the name --refspec (which is perfectly fine) is just an excuse to not fix the issues already present. -- Felipe Contreras -- 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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Am 07.12.2013 00:52, schrieb Junio C Hamano: > * kb/doc-exclude-directory-semantics (2013-11-07) 1 commit > - gitignore.txt: clarify recursive nature of excluded directories > > Originally merged to 'next' on 2013-11-13 > > Kicked back to 'pu' to replace with a newer reroll ($gmane/237814 > looked OK but there seems to have some loose ends in the > discussion). I'm unaware of any loose ends, could you clarify? Btw. $gmane/237814 seems to be a different topic, the version in next (and now in pu) was $gmane/237429. > * kb/fast-hashmap (2013-11-18) 14 commits > (merged to 'next' on 2013-12-06 at f90be3d) Damn, a day too late :-) I found these two glitches today...is a fixup patch OK or should I do a reroll (or separate patch on top)? Thanks, Karsten --- 8< --- Subject: [PATCH] fixup! add a hashtable implementation that supports O(1) removal Use 'unsigned int' for hash-codes everywhere. Extending 'struct hashmap_entry' with an int-sized member shouldn't waste memory on 64-bit systems. This is already documented in api-hashmap.txt, but needs '__attribute__((__packed__))' to work. Reduces e.g. struct name_entry { struct hashmap_entry ent; int namelen; char *name; }; from 32 to 24 bytes. Signed-off-by: Karsten Blees --- hashmap.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hashmap.h b/hashmap.h index f5b3b61..b64567b 100644 --- a/hashmap.h +++ b/hashmap.h @@ -15,7 +15,7 @@ extern unsigned int memihash(const void *buf, size_t len); /* data structures */ -struct hashmap_entry { +struct __attribute__((__packed__)) hashmap_entry { struct hashmap_entry *next; unsigned int hash; }; @@ -43,7 +43,7 @@ extern void hashmap_free(struct hashmap *map, int free_entries); /* hashmap_entry functions */ -static inline void hashmap_entry_init(void *entry, int hash) +static inline void hashmap_entry_init(void *entry, unsigned int hash) { struct hashmap_entry *e = entry; e->hash = hash; -- 1.8.5.1.178.g0a9afc1.dirty -- 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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Junio C Hamano writes: > * mm/mv-file-to-no-such-dir-with-slash (2013-12-04) 1 commit > - mv: let 'git mv file no-such-dir/' error out Why is it "stalled"? I think I took all remarks into account. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Junio C Hamano writes: > * jk/pack-bitmap (2013-11-18) 22 commits > - compat/mingw.h: Fix the MinGW and msvc builds > - pack-bitmap: implement optional name_hash cache > - t/perf: add tests for pack bitmaps > - t: add basic bitmap functionality tests > - count-objects: recognize .bitmap in garbage-checking > - repack: consider bitmaps when performing repacks > - repack: handle optional files created by pack-objects > - repack: turn exts array into array-of-struct > - repack: stop using magic number for ARRAY_SIZE(exts) > - pack-objects: implement bitmap writing > - rev-list: add bitmap mode to speed up object lists > - pack-objects: use bitmaps when packing objects > - pack-bitmap: add support for bitmap indexes > - documentation: add documentation for the bitmap format > - ewah: compressed bitmap implementation > - compat: add endianness helpers > - sha1_file: export `git_open_noatime` > - revision: allow setting custom limiter function > - pack-objects: factor out name_hash > - pack-objects: refactor the packing list > - revindex: export new APIs > - sha1write: make buffer const-correct > (this branch is tangled with jk/name-pack-after-byte-representation.) > > Borrows the bitmap index into packfiles from JGit to speed up > enumeration of objects involved in a commit range without having to > fully traverse the history. Peff can decide if he wants to reroll with my nits or not; either way I'm all for moving it forward and aiming for one of the next releases. -- Thomas Rast t...@thomasrast.ch -- 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: What's cooking in git.git (Dec 2013, #02; Fri, 6)
Junio C Hamano wrote: > * fc/transport-helper-fixes (2013-11-13) 12 commits > - remote-bzr: support the new 'force' option > - transport-helper: add support to delete branches > - fast-export: add support to delete refs > - fast-import: add support to delete refs > - transport-helper: add support for old:new refspec > - fast-export: add new --refspec option > - fast-export: improve argument parsing > - test-hg.sh: tests are now expected to pass > - transport-helper: check for 'forced update' message > - transport-helper: add 'force' to 'export' helpers > - transport-helper: don't update refs in dry-run > - transport-helper: mismerge fix > > Updates transport-helper, fast-import and fast-export to allow the > ref mapping and ref deletion in a way similar to the natively > supported transports. > > The option name "--refspec" needs to be rethought. It does not mean > what refspec usually means, even though it shares the same syntax > with refspec; calling it --refspec only because it shares the same > syntax is like calling it --asciistring and does not make sense. Not true. remote..fetch is a refspec, and doesn't specify what to fetch, so does the "refspec" capability in remote helpers, and so does the proposed --refspec option. It is exactly what it already means. -- Felipe Contreras -- 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
What's cooking in git.git (Dec 2013, #02; Fri, 6)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The tip of 'next' has been rewound, ejecting a few topics that used to be there. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [New Topics] * jn/git-gui-chmod+x (2013-11-25) 1 commit - git-gui: chmod +x po2msg, windows/git-gui.sh Parked here until I get the same change back from the upstream git-gui tree. * jn/gitk-chmod+x (2013-11-25) 1 commit - gitk: chmod +x po2msg Parked here until I get the same change back from the upstream gitk tree. * jk/name-pack-after-byte-representation (2013-12-05) 2 commits - pack-objects: name pack files after trailer hash - sha1write: make buffer const-correct (this branch is tangled with jk/pack-bitmap.) * nd/negative-pathspec (2013-12-06) 3 commits - pathspec.c: support adding prefix magic to a pathspec with mnemonic magic - Support pathspec magic :(exclude) and its short form :! - glossary-content.txt: rephrase magic signature part * nd/transport-positive-depth-only (2013-12-06) 1 commit - clone,fetch: catch non positive --depth option value * zk/difftool-counts (2013-12-06) 1 commit - difftool: display the number of files in the diff queue in the prompt -- [Graduated to "master"] * ak/submodule-foreach-quoting (2013-09-27) 1 commit (merged to 'next' on 2013-10-14 at d77c5f1) + submodule foreach: skip eval for more than one argument A behavior change, but maybe a worthwhile one: "git submodule foreach" was treating its arguments as part of a single command to be concatenated and passed to a shell, making writing buggy scripts too easy. This patch preserves the old "just pass it to the shell" behavior when a single argument is passed to 'git submodule foreach' and moves to a new "skip the shell and use the arguments passed unmolested" behavior when more than one argument is passed. The old behavior (always concatenating and passing to the shell) was similar to the 'ssh' command, while the new behavior (switching on the number of arguments) is what 'xterm -e' does. May need more thought to make sure this change is advertised well; scripts that used multiple arguments but added their own extra layer of quoting are broken, and the users need to adjust them. * bc/http-100-continue (2013-10-31) 3 commits (merged to 'next' on 2013-11-01 at e12ae23) + remote-curl: fix large pushes with GSSAPI + remote-curl: pass curl slot_results back through run_slot + http: return curl's AUTHAVAIL via slot_results Issue "100 Continue" responses to help use of GSS-Negotiate authentication scheme over HTTP transport when needed. * jc/bundle (2013-11-12) 1 commit (merged to 'next' on 2013-11-21 at 535b046) + bundle: use argv-array Code clean-up. * jc/merge-base-reflog (2013-10-29) 2 commits (merged to 'next' on 2013-11-01 at 6114764) + merge-base: teach "--fork-point" mode + merge-base: use OPT_CMDMODE and clarify the command line parsing Code the logic in "pull --rebase" that figures out a fork point from reflog entries in C. * jc/ref-excludes (2013-11-01) 5 commits (merged to 'next' on 2013-11-04 at fac1ed0) + rev-parse: introduce --exclude= to tame wildcards + rev-list --exclude: export add/clear-ref-exclusion and ref-excluded API + rev-list --exclude: tests + document --exclude option + revision: introduce --exclude= to tame wildcards People often wished a way to tell "git log --branches" (and "git log --remotes --not --branches") to exclude some local branches from the expansion of "--branches" (similarly for "--tags", "--all" and "--glob="). Now they have one. * jh/loose-object-dirs-creation-race (2013-10-28) 1 commit (merged to 'next' on 2013-11-01 at 3169b0f) + sha1_file.c:create_tmpfile(): Fix race when creating loose object dirs Two processes creating loose objects at the same time could have failed unnecessarily when they happened to have created objects whose names share the same first byte. * jk/remove-experimental-loose-object-support (2013-11-21) 1 commit (merged to 'next' on 2013-11-21 at d37bab7) + drop support for "experimental" loose objects Read-only support for experimental loose-object format, in which users could optionally choose to write in their loose objects for a short while between v1.4.3 to v1.5.3 era, has been dropped. * jk/replace-perl-in-built-scripts (2013-10-29) 1 commit (merged to 'next' on 2013-11-01 at 2384e29) + use @@PERL@@ in built scripts * jk/robustify-parse-commit (2013-10-24) 6 commits (merged to 'next' on 2013-11-01 at 2bfbaab) + checkout: do not die when leaving broken detached HEAD + use parse_commit_or_die instead of custom message + use parse_commit_or_die instea