Changeset: 24c8015f6a94 for MonetDB URL: https://dev.monetdb.org/hg/MonetDB/rev/24c8015f6a94 Modified Files: clients/Tests/exports.stable.out gdk/gdk.h gdk/gdk_bat.c gdk/gdk_bbp.c gdk/gdk_hash.c gdk/gdk_heap.c gdk/gdk_logger.c gdk/gdk_storage.c gdk/gdk_system.c gdk/gdk_utils.c monetdb5/modules/atoms/json.c sql/backends/monet5/UDF/capi/capi.c sql/storage/bat/bat_logger.c Branch: default Log Message:
Reduce mallocs: GDKfilepath now writes into a buffer passed by caller. diffs (truncated from 1762 to 300 lines): diff --git a/clients/Tests/exports.stable.out b/clients/Tests/exports.stable.out --- a/clients/Tests/exports.stable.out +++ b/clients/Tests/exports.stable.out @@ -282,7 +282,7 @@ bool GDKexiting(void); jmp_buf GDKfataljump; bit GDKfataljumpenable; str GDKfatalmsg; -char *GDKfilepath(int farmid, const char *dir, const char *nme, const char *ext); +gdk_return GDKfilepath(char *buf, size_t bufsize, int farmid, const char *dir, const char *nme, const char *ext) __attribute__((__access__(write_only, 1))); void GDKfree(void *blk); char *GDKgetbuf(void); unsigned GDKgetdebug(void); diff --git a/gdk/gdk.h b/gdk/gdk.h --- a/gdk/gdk.h +++ b/gdk/gdk.h @@ -1484,8 +1484,10 @@ gdk_export gdk_return BATsave(BAT *b) __attribute__((__warn_unused_result__)); #define NOFARM (-1) /* indicate to GDKfilepath to create relative path */ +#define MAXPATH 1024 /* maximum supported file path */ -gdk_export char *GDKfilepath(int farmid, const char *dir, const char *nme, const char *ext); +gdk_export gdk_return GDKfilepath(char *buf, size_t bufsize, int farmid, const char *dir, const char *nme, const char *ext) + __attribute__((__access__(write_only, 1))); gdk_export bool GDKinmemory(int farmid); gdk_export bool GDKembedded(void); gdk_export gdk_return GDKcreatedir(const char *nme); diff --git a/gdk/gdk_bat.c b/gdk/gdk_bat.c --- a/gdk/gdk_bat.c +++ b/gdk/gdk_bat.c @@ -2327,7 +2327,7 @@ static gdk_return backup_new(Heap *hp, bool lock) { int batret, bakret, ret = -1; - char *batpath, *bakpath; + char batpath[MAXPATH], bakpath[MAXPATH]; struct stat st; char *bak_filename = NULL; @@ -2336,9 +2336,8 @@ backup_new(Heap *hp, bool lock) else bak_filename = hp->filename; /* check for an existing X.new in BATDIR, BAKDIR and SUBDIR */ - batpath = GDKfilepath(hp->farmid, BATDIR, hp->filename, "new"); - bakpath = GDKfilepath(hp->farmid, BAKDIR, bak_filename, "new"); - if (batpath != NULL && bakpath != NULL) { + if (GDKfilepath(batpath, sizeof(batpath), hp->farmid, BATDIR, hp->filename, "new") == GDK_SUCCEED && + GDKfilepath(bakpath, sizeof(bakpath), hp->farmid, BAKDIR, bak_filename, "new") == GDK_SUCCEED) { /* file actions here interact with the global commits */ if (lock) BBPtmlock(); @@ -2364,8 +2363,6 @@ backup_new(Heap *hp, bool lock) if (lock) BBPtmunlock(); } - GDKfree(batpath); - GDKfree(bakpath); return ret ? GDK_FAIL : GDK_SUCCEED; } diff --git a/gdk/gdk_bbp.c b/gdk/gdk_bbp.c --- a/gdk/gdk_bbp.c +++ b/gdk/gdk_bbp.c @@ -894,7 +894,7 @@ BBPcheckbats(unsigned bbpversion) for (bat bid = 1, size = (bat) ATOMIC_GET(&BBPsize); bid < size; bid++) { struct stat statb; BAT *b; - char *path; + char path[MAXPATH]; b = BBP_desc(bid); if (b->batCacheid == 0 || b->ttype == TYPE_void) { @@ -902,20 +902,17 @@ BBPcheckbats(unsigned bbpversion) continue; } if (b->theap->free > 0) { - path = GDKfilepath(0, BATDIR, b->theap->filename, NULL); - if (path == NULL) + if (GDKfilepath(path, sizeof(path), 0, BATDIR, b->theap->filename, NULL) != GDK_SUCCEED) return GDK_FAIL; /* first check string offset heap with width, * then without */ if (MT_stat(path, &statb) < 0) { GDKsyserror("cannot stat file %s (expected size %zu)\n", path, b->theap->free); - GDKfree(path); return GDK_FAIL; } if ((size_t) statb.st_size < b->theap->free) { GDKerror("file %s too small (expected %zu, actual %zu)\n", path, b->theap->free, (size_t) statb.st_size); - GDKfree(path); return GDK_FAIL; } size_t hfree = b->theap->free; @@ -930,21 +927,17 @@ BBPcheckbats(unsigned bbpversion) (void) close(fd); } } - GDKfree(path); } if (b->tvheap != NULL && b->tvheap->free > 0) { - path = GDKfilepath(0, BATDIR, BBP_physical(b->batCacheid), "theap"); - if (path == NULL) + if (GDKfilepath(path, sizeof(path), 0, BATDIR, BBP_physical(b->batCacheid), "theap") != GDK_SUCCEED) return GDK_FAIL; if (MT_stat(path, &statb) < 0) { GDKsyserror("cannot stat file %s\n", path); - GDKfree(path); return GDK_FAIL; } if ((size_t) statb.st_size < b->tvheap->free) { GDKerror("file %s too small (expected %zu, actual %zu)\n", path, b->tvheap->free, (size_t) statb.st_size); - GDKfree(path); return GDK_FAIL; } size_t hfree = b->tvheap->free; @@ -959,7 +952,6 @@ BBPcheckbats(unsigned bbpversion) (void) close(fd); } } - GDKfree(path); } } return GDK_SUCCEED; @@ -1110,7 +1102,7 @@ BBPaddfarm(const char *dirname, uint32_t } BBPfarms[i].roles = rolemask; if ((rolemask & 1) == 0 && dirname != NULL) { - char *bbpdir; + char bbpdir[MAXPATH]; int j; for (j = 0; j < i; j++) @@ -1122,28 +1114,22 @@ BBPaddfarm(const char *dirname, uint32_t * don't find a BBP.dir there that * might belong to an existing * database */ - bbpdir = GDKfilepath(i, BATDIR, "BBP", "dir"); - if (bbpdir == NULL) { + if (GDKfilepath(bbpdir, sizeof(bbpdir), i, BATDIR, "BBP", "dir") != GDK_SUCCEED) { return GDK_FAIL; } if (MT_stat(bbpdir, &st) != -1 || errno != ENOENT) { - GDKfree(bbpdir); if (logerror) GDKerror("%s is a database\n", dirname); return GDK_FAIL; } - GDKfree(bbpdir); - bbpdir = GDKfilepath(i, BAKDIR, "BBP", "dir"); - if (bbpdir == NULL) { + if (GDKfilepath(bbpdir, sizeof(bbpdir), i, BAKDIR, "BBP", "dir") != GDK_SUCCEED) { return GDK_FAIL; } if (MT_stat(bbpdir, &st) != -1 || errno != ENOENT) { - GDKfree(bbpdir); if (logerror) GDKerror("%s is a database\n", dirname); return GDK_FAIL; } - GDKfree(bbpdir); } return GDK_SUCCEED; } @@ -1188,8 +1174,9 @@ static gdk_return fixhashashbat(BAT *b) { const char *nme = BBP_physical(b->batCacheid); - char *srcdir = GDKfilepath(NOFARM, BATDIR, nme, NULL); - if (srcdir == NULL) { + char srcdir[MAXPATH]; + + if (GDKfilepath(srcdir, sizeof(srcdir), NOFARM, BATDIR, nme, NULL) != GDK_SUCCEED) { TRC_CRITICAL(GDK, "GDKfilepath failed\n"); return GDK_FAIL; } @@ -1228,14 +1215,12 @@ fixhashashbat(BAT *b) BAKDIR, bnme, "tail", true) == GDK_SUCCEED) t = "tail"; else { - GDKfree(srcdir); TRC_CRITICAL(GDK, "cannot make backup of %s.tail\n", nme); return GDK_FAIL; } GDKclrerr(); if (GDKmove(b->theap->farmid, srcdir, bnme, "theap", BAKDIR, bnme, "theap", true) != GDK_SUCCEED) { - GDKfree(srcdir); TRC_CRITICAL(GDK, "cannot make backup of %s.theap\n", nme); return GDK_FAIL; } @@ -1245,7 +1230,6 @@ fixhashashbat(BAT *b) h1.dirty = false; strconcat_len(h1.filename, sizeof(h1.filename), filename, ".", t, NULL); if (HEAPload(&h1, filename, t, false) != GDK_SUCCEED) { - GDKfree(srcdir); TRC_CRITICAL(GDK, "loading old tail heap " "for BAT %d failed\n", b->batCacheid); return GDK_FAIL; @@ -1255,7 +1239,6 @@ fixhashashbat(BAT *b) vh1.dirty = false; strconcat_len(vh1.filename, sizeof(vh1.filename), filename, ".theap", NULL); if (HEAPload(&vh1, filename, "theap", false) != GDK_SUCCEED) { - GDKfree(srcdir); HEAPfree(&h1, false); TRC_CRITICAL(GDK, "loading old string heap " "for BAT %d failed\n", b->batCacheid); @@ -1268,7 +1251,6 @@ fixhashashbat(BAT *b) if (h2 == NULL || vh2 == NULL) { GDKfree(h2); GDKfree(vh2); - GDKfree(srcdir); HEAPfree(&h1, false); HEAPfree(&vh1, false); TRC_CRITICAL(GDK, "allocating new heaps " @@ -1280,7 +1262,6 @@ fixhashashbat(BAT *b) if (HEAPalloc(h2, b->batCapacity, b->twidth) != GDK_SUCCEED) { GDKfree(h2); GDKfree(vh2); - GDKfree(srcdir); HEAPfree(&h1, false); HEAPfree(&vh1, false); TRC_CRITICAL(GDK, "allocating new tail heap " @@ -1294,7 +1275,6 @@ fixhashashbat(BAT *b) strconcat_len(vh2->filename, sizeof(vh2->filename), nme, ".theap", NULL); strHeap(vh2, b->batCapacity); if (vh2->base == NULL) { - GDKfree(srcdir); HEAPfree(&h1, false); HEAPfree(&vh1, false); HEAPfree(h2, false); @@ -1336,7 +1316,6 @@ fixhashashbat(BAT *b) HEAPdecref(h2, false); HEAPdecref(b->tvheap, false); b->tvheap = ovh; - GDKfree(srcdir); TRC_CRITICAL(GDK, "storing string value " "for BAT %d failed\n", b->batCacheid); return GDK_FAIL; @@ -1372,14 +1351,12 @@ fixhashashbat(BAT *b) HEAPdecref(h2, false); HEAPdecref(b->tvheap, false); b->tvheap = ovh; - GDKfree(srcdir); TRC_CRITICAL(GDK, "saving heap failed\n"); return GDK_FAIL; } if (HEAPsave(b->tvheap, nme, "theap", true, b->tvheap->free, &b->theaplock) != GDK_SUCCEED) { HEAPfree(b->tvheap, false); b->tvheap = ovh; - GDKfree(srcdir); TRC_CRITICAL(GDK, "saving string heap failed\n"); return GDK_FAIL; } @@ -1388,7 +1365,6 @@ fixhashashbat(BAT *b) HEAPfree(h2, false); HEAPdecref(ovh, false); HEAPfree(b->tvheap, false); - GDKfree(srcdir); return GDK_SUCCEED; } @@ -1414,9 +1390,9 @@ static gdk_return jsonupgradebat(BAT *b, json_storage_conversion fixJSONStorage) { const char *nme = BBP_physical(b->batCacheid); - char *srcdir = GDKfilepath(NOFARM, BATDIR, nme, NULL); - - if (srcdir == NULL) { + char srcdir[MAXPATH]; + + if (GDKfilepath(srcdir, sizeof(srcdir), NOFARM, BATDIR, nme, NULL) != GDK_SUCCEED) { TRC_CRITICAL(GDK, "GDKfilepath failed\n"); return GDK_FAIL; } @@ -1444,13 +1420,11 @@ jsonupgradebat(BAT *b, json_storage_conv /* backup the current heaps */ if (GDKmove(b->theap->farmid, srcdir, bnme, "tail", BAKDIR, bnme, "tail", false) != GDK_SUCCEED) { - GDKfree(srcdir); TRC_CRITICAL(GDK, "cannot make backup of %s.tail\n", nme); return GDK_FAIL; } if (GDKmove(b->theap->farmid, srcdir, bnme, "theap", BAKDIR, bnme, "theap", true) != GDK_SUCCEED) { - GDKfree(srcdir); TRC_CRITICAL(GDK, "cannot make backup of %s.theap\n", nme); return GDK_FAIL; } @@ -1461,7 +1435,6 @@ jsonupgradebat(BAT *b, json_storage_conv h1.dirty = false; strconcat_len(h1.filename, sizeof(h1.filename), filename, ".tail", NULL); if (HEAPload(&h1, filename, "tail", false) != GDK_SUCCEED) { - GDKfree(srcdir); TRC_CRITICAL(GDK, "loading old tail heap " "for BAT %d failed\n", b->batCacheid); return GDK_FAIL; @@ -1472,7 +1445,6 @@ jsonupgradebat(BAT *b, json_storage_conv vh1.dirty = false; _______________________________________________ checkin-list mailing list -- checkin-list@monetdb.org To unsubscribe send an email to checkin-list-le...@monetdb.org