Changeset: 49cc8f100cb4 for MonetDB
URL: http://dev.monetdb.org/hg/MonetDB?cmd=changeset;node=49cc8f100cb4
Modified Files:
gdk/gdk_heap.c
gdk/gdk_private.h
gdk/gdk_utils.c
Branch: Feb2013
Log Message:
Get rid of the heap cache.
This fixes bug 3376 and relates to bug 3323.
In bug 3323, comment 15, I already remarked that using the heap cache
is actually slower on Linux than not using the heap cache.
In addition to this, using the heap cache can be very detrimental as
witnessed by bug 3376. What happens there is that a new string bat is
created and then appended to. The appended string bat is big enough
so that the offset heap needs to be memory mapped, so the heap cache
is used to find a usable heap. In this case, there is only one file
there, so it is used. BATappend then needs to widen the offset heap,
so it calls GDKupgradevarheap. That function doubles (or in this
case, quadruples) the size of the heap it was given since it doesn't
know the required BAT capacity. Then the BAT is freed and the heap
returned to the cache. This cycle repeats a few times, and this
repetition causes the heap to grow exponentially, eventually leading
to the errors seen in the bug report.
diffs (268 lines):
diff --git a/gdk/gdk_heap.c b/gdk/gdk_heap.c
--- a/gdk/gdk_heap.c
+++ b/gdk/gdk_heap.c
@@ -59,172 +59,21 @@
#include "gdk.h"
#include "gdk_private.h"
-/* The heap cache should reduce mmap/munmap calls which are very
- * expensive. Instead we try to reuse mmap's. This however requires
- * file renames. The cache has a limited size!
- */
-
-#define HEAP_CACHE_SIZE 5
-
-typedef struct heap_cache_e {
- void *base;
- size_t maxsz;
- char fn[8]; /* tmp file name */
-} heap_cache_e;
-
-typedef struct heap_cache {
- int sz;
- int used;
- heap_cache_e *hc;
-} heap_cache;
-
-static heap_cache hc;
-static MT_Lock HEAPcacheLock;
-
-void
-HEAPcacheInit(void)
-{
- MT_lock_init(&HEAPcacheLock, "HEAPcache_init");
-#if HEAP_CACHE_SIZE > 0
- MT_lock_set(&HEAPcacheLock, "HEAPcache_init");
- assert(hc.sz == 0);
- hc.used = 0;
- hc.hc = GDKmalloc(sizeof(heap_cache_e) * HEAP_CACHE_SIZE);
- if (hc.hc != NULL) {
- int i;
-
- hc.sz = HEAP_CACHE_SIZE;
- GDKcreatedir(HCDIR DIR_SEP_STR);
- /* clean old leftovers */
- for (i = 0; i < HEAP_CACHE_SIZE; i++) {
- char fn[8];
-
- snprintf(fn, sizeof(fn), "%d", i);
- GDKunlink(HCDIR, fn, NULL);
- }
- }
- MT_lock_unset(&HEAPcacheLock, "HEAPcache_init");
-#endif
-}
-
-static int
-HEAPcacheAdd(void *base, size_t maxsz, char *fn, storage_t storage, int
free_file)
-{
- int added = 0;
-
-
- MT_lock_set(&HEAPcacheLock, "HEAPcache_init");
- if (free_file && fn && storage == STORE_MMAP && hc.used < hc.sz) {
- heap_cache_e *e = hc.hc + hc.used;
-
- e->base = base;
- e->maxsz = maxsz;
- snprintf(e->fn, sizeof(e->fn), "%d", hc.used);
- GDKunlink(HCDIR, e->fn, NULL);
- added = 1;
- if (GDKmove(BATDIR, fn, NULL, HCDIR, e->fn, NULL) < 0) {
- /* try to create the directory, if that was
- * the problem */
- char path[PATHLENGTH];
-
- GDKfilepath(path, HCDIR, e->fn, NULL);
- GDKcreatedir(path);
- if (GDKmove(BATDIR, fn, NULL, HCDIR, e->fn, NULL) < 0)
- added = 0;
- }
- if (added)
- hc.used++;
- }
- MT_lock_unset(&HEAPcacheLock, "HEAPcache_init");
- if (!added)
- return GDKmunmap(base, maxsz);
- HEAPDEBUG fprintf(stderr, "#HEAPcacheAdd (%s) " SZFMT " " PTRFMT " %d
%d %d\n", fn, maxsz, PTRFMTCAST base, (int) storage, free_file, hc.used);
- return 0;
-}
-
static void *
-HEAPcacheFind(size_t *maxsz, char *fn, storage_t mode)
+HEAPcreatefile(size_t *maxsz, char *fn, storage_t mode)
{
size_t size = *maxsz;
void *base = NULL;
+ int fd;
size = (*maxsz + (size_t) 0xFFFF) & ~ (size_t) 0xFFFF; /* round up to
64k */
- MT_lock_set(&HEAPcacheLock, "HEAPcache_init");
- if (mode == STORE_MMAP && hc.used > 0) {
- int i;
- heap_cache_e *e = NULL;
- size_t cursz = 0;
-
- HEAPDEBUG fprintf(stderr, "#HEAPcacheFind (%s)" SZFMT " %d
%d\n", fn, size, (int) mode, hc.used);
-
- /* find best match: prefer smallest larger than or
- * equal to requested, otherwise largest smaller than
- * requested */
- for (i = 0; i < hc.used; i++) {
- if ((hc.hc[i].maxsz >= size &&
- (e == NULL || hc.hc[i].maxsz < cursz || cursz <
size)) ||
- (hc.hc[i].maxsz < size &&
- cursz < size &&
- hc.hc[i].maxsz > cursz)) {
- e = hc.hc + i;
- cursz = e->maxsz;
- }
- }
- if (e != NULL && e->maxsz < size) {
- /* resize file ? */
- long_str fn;
-
- GDKfilepath(fn, HCDIR, e->fn, NULL);
- base = MT_mremap(fn, MMAP_READ | MMAP_WRITE,
- e->base, e->maxsz, &size);
- if (base == NULL) {
- /* extending may have failed */
- e = NULL;
- } else {
- e->base = base;
- e->maxsz = size;
- }
- }
- if (e != NULL) {
- /* move cached heap to its new location */
- base = e->base;
- size = e->maxsz;
- if (GDKmove(HCDIR, e->fn, NULL, BATDIR, fn, NULL) < 0) {
- /* try to create the directory, if
- * that was the problem */
- char path[PATHLENGTH];
-
- GDKfilepath(path, BATDIR, fn, NULL);
- GDKcreatedir(path);
- if (GDKmove(HCDIR, e->fn, NULL, BATDIR, fn,
NULL) < 0)
- e = NULL;
- }
- }
- if (e != NULL) {
- hc.used--;
- i = (int) (e - hc.hc);
- if (i < hc.used) {
- e->base = hc.hc[hc.used].base;
- e->maxsz = hc.hc[hc.used].maxsz;
- GDKmove(HCDIR, hc.hc[hc.used].fn, NULL, HCDIR,
e->fn, NULL);
- }
- }
+ fd = GDKfdlocate(fn, "wb", NULL);
+ if (fd >= 0) {
+ close(fd);
+ base = GDKload(fn, NULL, size, size, mode);
+ if (base)
+ *maxsz = size;
}
- MT_lock_unset(&HEAPcacheLock, "HEAPcache_init");
- if (base == NULL) {
- int fd = GDKfdlocate(fn, "wb", NULL);
-
- if (fd >= 0) {
- close(fd);
- base = GDKload(fn, NULL, size, size, mode);
- if (base)
- *maxsz = size;
- return base;
- }
- } else
- HEAPDEBUG fprintf(stderr, "#HEAPcacheFind (%s) re-used\n", fn);
- if (base)
- *maxsz = size;
return base;
}
@@ -294,7 +143,7 @@ HEAPalloc(Heap *h, size_t nitems, size_t
if (stat(nme, &st) != 0) {
h->storage = STORE_MMAP;
- h->base = HEAPcacheFind(&h->size, of, h->storage);
+ h->base = HEAPcreatefile(&h->size, of, h->storage);
h->maxsize = h->size;
h->filename = of;
} else {
@@ -429,7 +278,7 @@ HEAPextend(Heap *h, size_t size)
goto failed;
}
sprintf(h->filename, "%s.%s", nme, ext);
- h->base = HEAPcacheFind(&h->size, h->filename,
STORE_MMAP);
+ h->base = HEAPcreatefile(&h->size, h->filename,
STORE_MMAP);
if (h->base) {
h->maxsize = h->size;
h->newstorage = h->storage = STORE_MMAP;
@@ -658,8 +507,8 @@ HEAPcopy(Heap *dst, Heap *src)
* Is now called even on heaps without memory, just to free the
* pre-allocated filename. simple: alloc and copy.
*/
-static int
-HEAPfree_internal(Heap *h, int free_file)
+int
+HEAPfree(Heap *h)
{
if (h->base) {
if (h->storage == STORE_MEM) { /* plain memory */
@@ -669,8 +518,7 @@ HEAPfree_internal(Heap *h, int free_file
PTRFMTCAST h->base);
GDKfree(h->base);
} else { /* mapped file, or STORE_PRIV */
- int ret = HEAPcacheAdd(h->base, h->size, h->filename,
- h->storage, free_file);
+ int ret = GDKmunmap(h->base, h->size);
if (ret < 0) {
GDKsyserror("HEAPfree: %s was not mapped\n",
@@ -691,12 +539,6 @@ HEAPfree_internal(Heap *h, int free_file
return 0;
}
-int
-HEAPfree(Heap *h)
-{
- return HEAPfree_internal(h, 0);
-}
-
/*
* @- HEAPload
*
@@ -843,7 +685,7 @@ HEAPdelete(Heap *h, const char *o, const
return 0;
}
if (h->base)
- HEAPfree_internal(h, 1);
+ HEAPfree(h);
if (h->copied) {
return 0;
}
diff --git a/gdk/gdk_private.h b/gdk/gdk_private.h
--- a/gdk/gdk_private.h
+++ b/gdk/gdk_private.h
@@ -69,7 +69,6 @@ int HASHgonebad(BAT *b, const void *v);
BUN HASHmask(BUN cnt);
Hash *HASHnew(Heap *hp, int tpe, BUN size, BUN mask);
int HEAPalloc(Heap *h, size_t nitems, size_t itemsize);
-void HEAPcacheInit(void);
int HEAP_check(Heap *h, HeapRepair *hr);
int HEAPdelete(Heap *h, const char *o, const char *ext);
void HEAP_init(Heap *heap, int tpe);
diff --git a/gdk/gdk_utils.c b/gdk/gdk_utils.c
--- a/gdk/gdk_utils.c
+++ b/gdk/gdk_utils.c
@@ -1016,8 +1016,6 @@ GDKinit(opt *set, int setlen)
GDKremovedir(DELDIR);
BBPinit();
- HEAPcacheInit();
-
GDKkey = BATnew(TYPE_void, TYPE_str, 100);
GDKval = BATnew(TYPE_void, TYPE_str, 100);
if (GDKkey == NULL)
_______________________________________________
checkin-list mailing list
[email protected]
https://www.monetdb.org/mailman/listinfo/checkin-list
