Am 08.12.2013 11:20, schrieb Thomas Rast:
> Karsten Blees <[email protected]> writes:
>
>> Am 07.12.2013 23:23, schrieb Thomas Rast:
>>> Karsten Blees <[email protected]> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
