[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: another hash cleanup
From: |
Jim Meyering |
Subject: |
Re: another hash cleanup |
Date: |
Fri, 19 Jun 2009 15:06:41 +0200 |
Eric Blake wrote:
> According to Jim Meyering on 6/19/2009 6:02 AM:
>>> But, come to think of it, why are we even malloc'ing the new_table at all
>>> in this code path? Maybe the better technical approach would be factoring
>>> out the table size computation from hash_initialize, and have both
>>> hash_initialize and hash_rehash first compute the needed size and later do
>>> the malloc, so that hash_rehash doesn't even malloc in the first place if
>>> the size computation shows it isn't necessary.
>>
>> Yes, I think that would be better still.
>
> Like so, along with an overflow bug fix. OK to apply?
...
> +2009-06-19 Eric Blake <address@hidden>
> +
> + hash: reduce memory pressure in hash_rehash no-op case
> + * lib/hash.c (next_prime): Avoid overflow.
> + (hash_initialize): Factor bucket size computation...
> + (compute_bucket_size): ...into new helper function.
> + (hash_rehash): Use new function and open coding to reduce memory
> + pressure, and avoids a memory leak in USE_OBSTACK code.
> + Reported by Jim Meyering.
Looks good, but I haven't tested.
Ok, presuming you have.
One minor suggested change:
> diff --git a/lib/hash.c b/lib/hash.c
...
> @@ -847,25 +857,33 @@ hash_find_entry (Hash_table *table, const void *entry,
> bool
> hash_rehash (Hash_table *table, size_t candidate)
> {
> + Hash_table storage;
> Hash_table *new_table;
> struct hash_entry *bucket;
> struct hash_entry *cursor;
> struct hash_entry *next;
> + size_t new_size;
>
> - new_table = hash_initialize (candidate, table->tuning, table->hasher,
> - table->comparator, table->data_freer);
> - if (new_table == NULL)
> + new_size = compute_bucket_size (candidate, table->tuning);
Please combine the declaration and assignment:
size_t new_size = compute_bucket_size (candidate, table->tuning);
- Re: hash resizing bug, (continued)
- Re: hash resizing bug, Ben Pfaff, 2009/06/18
- another hash cleanup (was: hash resizing bug), Eric Blake, 2009/06/18
- Re: another hash cleanup, Jim Meyering, 2009/06/18
- Re: another hash cleanup, Eric Blake, 2009/06/18
- Re: another hash cleanup, Jim Meyering, 2009/06/18
- Re: another hash cleanup, Eric Blake, 2009/06/18
- Re: another hash cleanup, Jim Meyering, 2009/06/19
- Re: another hash cleanup, Eric Blake, 2009/06/19
- Re: another hash cleanup, Jim Meyering, 2009/06/19
- Re: another hash cleanup, Eric Blake, 2009/06/19
- Re: another hash cleanup,
Jim Meyering <=
- Re: another hash cleanup, Eric Blake, 2009/06/19
- Re: another hash cleanup, Jim Meyering, 2009/06/19
- Re: another hash cleanup, Eric Blake, 2009/06/19
- Re: another hash cleanup, Jim Meyering, 2009/06/19
- Re: another hash cleanup, Jim Meyering, 2009/06/19