Changeset: 60c9b0a73ffc for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB/rev/60c9b0a73ffc
Modified Files:
gdk/gdk.h
gdk/gdk_bbp.c
gdk/gdk_hash.c
gdk/gdk_heap.c
gdk/gdk_imprints.c
gdk/gdk_orderidx.c
gdk/gdk_private.h
gdk/gdk_storage.c
Branch: Jul2021
Log Message:
Don't keep lock while saving bat, use iterator instead.
diffs (truncated from 393 to 300 lines):
diff --git a/gdk/gdk.h b/gdk/gdk.h
--- a/gdk/gdk.h
+++ b/gdk/gdk.h
@@ -918,6 +918,7 @@ typedef struct BATiter {
uint8_t shift;
int8_t type;
oid tseq;
+ BUN hfree, vhfree;
union {
oid tvid;
bool tmsk;
@@ -944,6 +945,8 @@ bat_iterator(BAT *b)
.shift = b->tshift,
.type = b->ttype,
.tseq = b->tseqbase,
+ .hfree = b->theap->free,
+ .vhfree = b->tvheap ? b->tvheap->free : 0,
#ifndef NDEBUG
.locked = true,
#endif
diff --git a/gdk/gdk_bbp.c b/gdk/gdk_bbp.c
--- a/gdk/gdk_bbp.c
+++ b/gdk/gdk_bbp.c
@@ -3293,25 +3293,22 @@ BBPsync(int cnt, bat *restrict subcommit
while (++idx < cnt) {
bat i = subcommit ? subcommit[idx] : idx;
- BAT *d = BBP_desc(i);
-
- if (d)
- MT_lock_set(&d->theaplock);
+ /* BBP_desc(i) may be NULL */
+ BATiter bi = bat_iterator(BBP_desc(i));
+
if (BBP_status(i) & BBPPERSISTENT) {
BAT *b = dirty_bat(&i, subcommit != NULL);
if (i <= 0) {
- if (d)
- MT_lock_unset(&d->theaplock);
+ bat_iterator_end(&bi);
break;
}
if (b)
- ret = BATsave_locked(b);
+ ret = BATsave_locked(b, &bi);
}
if (ret == GDK_SUCCEED) {
n = BBPdir_step(i, sizes ? sizes[idx] :
BUN_NONE, n, buf, sizeof(buf), &obbpf, nbbpf);
}
- if (d)
- MT_lock_unset(&d->theaplock);
+ bat_iterator_end(&bi);
if (n == -2)
break;
/* we once again have a saved heap */
diff --git a/gdk/gdk_hash.c b/gdk/gdk_hash.c
--- a/gdk/gdk_hash.c
+++ b/gdk/gdk_hash.c
@@ -605,8 +605,8 @@ BAThashsave_intern(BAT *b, bool dosync)
* mean time */
if (!b->theap->dirty &&
((size_t *) h->heapbckt.base)[4] == BATcount(b) &&
- HEAPsave(&h->heaplink, h->heaplink.filename, NULL, dosync)
== GDK_SUCCEED &&
- HEAPsave(hp, hp->filename, NULL, dosync) == GDK_SUCCEED) {
+ HEAPsave(&h->heaplink, h->heaplink.filename, NULL, dosync,
h->heaplink.free) == GDK_SUCCEED &&
+ HEAPsave(hp, hp->filename, NULL, dosync, hp->free) ==
GDK_SUCCEED) {
h->heaplink.dirty = false;
hp->dirty = false;
rc = HASHfix(h, true, dosync);
diff --git a/gdk/gdk_heap.c b/gdk/gdk_heap.c
--- a/gdk/gdk_heap.c
+++ b/gdk/gdk_heap.c
@@ -72,7 +72,7 @@ HEAPcreatefile(int farmid, size_t *maxsz
}
static gdk_return HEAPload_intern(Heap *h, const char *nme, const char *ext,
const char *suffix, bool trunc);
-static gdk_return HEAPsave_intern(Heap *h, const char *nme, const char *ext,
const char *suffix, bool dosync);
+static gdk_return HEAPsave_intern(Heap *h, const char *nme, const char *ext,
const char *suffix, bool dosync, BUN free);
static char *
decompose_filename(str nme)
@@ -202,6 +202,9 @@ HEAPalloc(Heap *h, size_t nitems, size_t
gdk_return
HEAPextend(Heap *h, size_t size, bool mayshare)
{
+ if (size <= h->size)
+ return GDK_SUCCEED; /* nothing to do */
+
char nme[sizeof(h->filename)], *ext;
const char *failure = "None";
@@ -212,9 +215,6 @@ HEAPextend(Heap *h, size_t size, bool ma
strcpy_len(nme, h->filename, sizeof(nme));
ext = decompose_filename(nme);
}
- if (size <= h->size)
- return GDK_SUCCEED; /* nothing to do */
-
failure = "size > h->size";
if (h->storage != STORE_MEM) {
@@ -313,7 +313,7 @@ HEAPextend(Heap *h, size_t size, bool ma
failure = "h->storage == STORE_MEM && can_map
&& fd >= 0 && HEAPload() != GDK_SUCCEED";
/* couldn't allocate, now first save data to
* file */
- if (HEAPsave_intern(&bak, nme, ext, ".tmp",
false) != GDK_SUCCEED) {
+ if (HEAPsave_intern(&bak, nme, ext, ".tmp",
false, bak.free) != GDK_SUCCEED) {
failure = "h->storage == STORE_MEM &&
can_map && fd >= 0 && HEAPsave_intern() != GDK_SUCCEED";
goto failed;
}
@@ -839,7 +839,7 @@ HEAPload(Heap *h, const char *nme, const
* safe on stable storage.
*/
static gdk_return
-HEAPsave_intern(Heap *h, const char *nme, const char *ext, const char *suffix,
bool dosync)
+HEAPsave_intern(Heap *h, const char *nme, const char *ext, const char *suffix,
bool dosync, BUN free)
{
storage_t store = h->newstorage;
long_str extension;
@@ -849,13 +849,13 @@ HEAPsave_intern(Heap *h, const char *nme
GDKerror("no heap to save\n");
return GDK_FAIL;
}
- if (h->free == 0) {
+ if (free == 0) {
/* nothing to see, please move on */
h->wasempty = true;
TRC_DEBUG(HEAP,
"not saving: "
"(%s.%s,storage=%d,free=%zu,size=%zu,dosync=%s)\n",
- nme?nme:"", ext, (int) h->newstorage, h->free,
h->size,
+ nme?nme:"", ext, (int) h->newstorage, free, h->size,
dosync?"true":"false");
return GDK_SUCCEED;
}
@@ -870,18 +870,21 @@ HEAPsave_intern(Heap *h, const char *nme
}
TRC_DEBUG(HEAP,
"(%s.%s,storage=%d,free=%zu,size=%zu,dosync=%s)\n",
- nme?nme:"", ext, (int) h->newstorage, h->free, h->size,
+ nme?nme:"", ext, (int) h->newstorage, free, h->size,
dosync?"true":"false");
- rc = GDKsave(h->farmid, nme, ext, h->base, h->free, store, dosync);
+ h->dirty = free != h->free;
+ rc = GDKsave(h->farmid, nme, ext, h->base, free, store, dosync);
if (rc == GDK_SUCCEED)
h->wasempty = false;
+ else
+ h->dirty = true;
return rc;
}
gdk_return
-HEAPsave(Heap *h, const char *nme, const char *ext, bool dosync)
+HEAPsave(Heap *h, const char *nme, const char *ext, bool dosync, BUN free)
{
- return HEAPsave_intern(h, nme, ext, ".new", dosync);
+ return HEAPsave_intern(h, nme, ext, ".new", dosync, free);
}
/*
diff --git a/gdk/gdk_imprints.c b/gdk/gdk_imprints.c
--- a/gdk/gdk_imprints.c
+++ b/gdk/gdk_imprints.c
@@ -389,7 +389,7 @@ BATimpsync(void *arg)
MT_lock_set(&b->batIdxLock);
if ((imprints = b->timprints) != NULL) {
Heap *hp = &imprints->imprints;
- if (HEAPsave(hp, hp->filename, NULL, true) == GDK_SUCCEED) {
+ if (HEAPsave(hp, hp->filename, NULL, true, hp->free) ==
GDK_SUCCEED) {
if (hp->storage == STORE_MEM) {
if ((fd = GDKfdlocate(hp->farmid, hp->filename,
"rb+", NULL)) >= 0) {
/* add version number */
diff --git a/gdk/gdk_orderidx.c b/gdk/gdk_orderidx.c
--- a/gdk/gdk_orderidx.c
+++ b/gdk/gdk_orderidx.c
@@ -25,7 +25,7 @@ BATidxsync(void *arg)
MT_lock_set(&b->batIdxLock);
if ((hp = b->torderidx) != NULL) {
- if (HEAPsave(hp, hp->filename, NULL, true) == GDK_SUCCEED) {
+ if (HEAPsave(hp, hp->filename, NULL, true, hp->free) ==
GDK_SUCCEED) {
if (hp->storage == STORE_MEM) {
if ((fd = GDKfdlocate(hp->farmid, hp->filename,
"rb+", NULL)) >= 0) {
((oid *) hp->base)[0] |= (oid) 1 << 24;
diff --git a/gdk/gdk_private.h b/gdk/gdk_private.h
--- a/gdk/gdk_private.h
+++ b/gdk/gdk_private.h
@@ -81,7 +81,7 @@ void BATrmprop(BAT *b, enum prop_t idx)
__attribute__((__visibility__("hidden")));
void BATrmprop_nolock(BAT *b, enum prop_t idx)
__attribute__((__visibility__("hidden")));
-gdk_return BATsave_locked(BAT *bd)
+gdk_return BATsave_locked(BAT *bd, BATiter *bi)
__attribute__((__visibility__("hidden")));
void BATsetdims(BAT *b)
__attribute__((__visibility__("hidden")));
@@ -208,7 +208,7 @@ gdk_return HEAPload(Heap *h, const char
__attribute__((__visibility__("hidden")));
void HEAP_recover(Heap *, const var_t *, BUN)
__attribute__((__visibility__("hidden")));
-gdk_return HEAPsave(Heap *h, const char *nme, const char *ext, bool dosync)
+gdk_return HEAPsave(Heap *h, const char *nme, const char *ext, bool dosync,
BUN free)
__attribute__((__warn_unused_result__))
__attribute__((__visibility__("hidden")));
gdk_return HEAPshrink(Heap *h, size_t size)
diff --git a/gdk/gdk_storage.c b/gdk/gdk_storage.c
--- a/gdk/gdk_storage.c
+++ b/gdk/gdk_storage.c
@@ -758,51 +758,56 @@ BATmsync(BAT *b)
#endif /* DISABLE_MSYNC */
}
+static inline const char *
+gettailnamebi(const BATiter *bi)
+{
+ if (bi->type != TYPE_str)
+ return "tail";
+ switch (bi->width) {
+ case 1:
+ return "tail1";
+ case 2:
+ return "tail2";
+#if SIZEOF_VAR_T == 8
+ case 4:
+ return "tail4";
+#endif
+ default:
+ return "tail";
+ }
+}
+
gdk_return
-BATsave_locked(BAT *bd)
+BATsave_locked(BAT *b, BATiter *bi)
{
gdk_return err = GDK_SUCCEED;
const char *nme;
- bool dosync = (BBP_status(bd->batCacheid) & BBPPERSISTENT) != 0;
+ bool dosync = (BBP_status(b->batCacheid) & BBPPERSISTENT) != 0;
- assert(!GDKinmemory(bd->theap->farmid));
- BATcheck(bd, GDK_FAIL);
+ assert(!GDKinmemory(b->theap->farmid));
+ BATcheck(b, GDK_FAIL);
- assert(bd->batCacheid > 0);
+ assert(b->batCacheid > 0);
/* views cannot be saved, but make an exception for
* force-remapped views */
- if (isVIEW(bd)) {
- GDKerror("%s is a view on %s; cannot be saved\n", BATgetId(bd),
BBP_logical(VIEWtparent(bd)));
+ if (isVIEW(b)) {
+ GDKerror("%s is a view on %s; cannot be saved\n", BATgetId(b),
BBP_logical(VIEWtparent(b)));
return GDK_FAIL;
}
- if (!BATdirty(bd)) {
+ if (!BATdirty(b)) {
return GDK_SUCCEED;
}
- /* copy the descriptor to a local variable in order to let our
- * messing in the BAT descriptor not affect other threads that
- * only read it. */
- BAT bs = *bd;
- BAT *b = &bs;
- Heap hs = *bd->theap;
- HEAPincref(&hs);
- b->theap = &hs;
- Heap vhs;
- if (b->tvheap) {
- vhs = *bd->tvheap;
- HEAPincref(&vhs);
- b->tvheap = &vhs;
- }
-
/* start saving data */
nme = BBP_physical(b->batCacheid);
- if (b->ttype != TYPE_void && b->theap->base == NULL) {
- assert(BBP_status(bd->batCacheid) & BBPSWAPPED);
+ const char *tail = gettailnamebi(bi);
+ if (bi->type != TYPE_void && bi->h->base == NULL) {
+ assert(BBP_status(b->batCacheid) & BBPSWAPPED);
if (dosync && !(GDKdebug & NOSYNCMASK)) {
- int fd = GDKfdlocate(b->theap->farmid, nme, "rb+",
gettailname(b));
+ int fd = GDKfdlocate(bi->h->farmid, nme, "rb+", tail);
if (fd < 0) {
- GDKsyserror("cannot open file %s.%s for
sync\n", nme,
- gettailname(b));
+ GDKsyserror("cannot open file %s.%s for sync\n",
+ nme, tail);
err = GDK_FAIL;
} else {
if (
@@ -814,12 +819,12 @@ BATsave_locked(BAT *bd)
fsync(fd) < 0
#endif
)
- GDKsyserror("sync failed for %s.%s\n",
nme,
- gettailname(b));
_______________________________________________
checkin-list mailing list
[email protected]
https://www.monetdb.org/mailman/listinfo/checkin-list
