Skip to content

Commit 15753e2

Browse files
committed
dhash: no need to keep previous table
The hash can only be enlarged from (1) a single-thread context, or (2) under a critical section, so there is no need to worry about other threads getting a use-after-free due to a reallocation. This should halve the memory use by the hash table.
1 parent c2b5c67 commit 15753e2

File tree

1 file changed

+9
-19
lines changed

1 file changed

+9
-19
lines changed

src/hash.c

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -99,14 +99,13 @@ typedef struct dhashtab_ {
9999
dhashtab public; // must be at offset 0
100100
size_t size, used, limit;
101101
uintptr_t multiplier;
102-
struct hash_pair *table, *previous;
102+
struct hash_pair *table;
103103
} dhashtab_;
104104

105105
static void dhash_finalizer(SEXP dhash) {
106106
dhashtab_ * self = R_ExternalPtrAddr(dhash);
107107
if (!self) return;
108108
R_ClearExternalPtr(dhash);
109-
free(self->previous);
110109
free(self->table);
111110
free(self);
112111
}
@@ -162,23 +161,14 @@ static void dhash_enlarge(dhashtab_ * self) {
162161
}
163162
}
164163
}
165-
// Not trying to protect from calls to _set -> _enlarge from other threads!
166-
// Writes only come from a critical section, so two threads will not attempt to enlarge at the same time.
167-
// What we have to prevent is yanking the self->table from under a different thread reading it right now.
168-
free(self->previous);
169-
struct hash_pair * previous = self->table;
170-
dhashtab public = self->public;
171-
size_t used = self->used, limit = self->limit*2;
172-
*self = (dhashtab_){
173-
.public = public,
174-
.size = new_size,
175-
.used = used,
176-
.limit = limit,
177-
.multiplier = new_multiplier,
178-
.table = new,
179-
.previous = previous,
180-
};
181-
#pragma omp flush // no locking or atomic access! this is bad
164+
// This is thread-unsafe, but this function is only called either from a single-thread context, or
165+
// from under an OpenMP critical section, so there's no reason to worry about another thread
166+
// getting a use-after-free. They are all sleeping.
167+
self->size = new_size;
168+
self->limit *= 2;
169+
self->multiplier = new_multiplier;
170+
self->table = new;
171+
#pragma omp flush
182172
}
183173

184174
void dhash_set(dhashtab * h, SEXP key, R_xlen_t value) {

0 commit comments

Comments
 (0)