Changeset: f4cb487b1b1d for MonetDB URL: https://dev.monetdb.org/hg/MonetDB?cmd=changeset;node=f4cb487b1b1d Modified Files: gdk/gdk_align.c gdk/gdk_bat.c gdk/gdk_batop.c Branch: Jun2020 Log Message:
Reimplemented VIEWreset to be more careful when things go wrong. diffs (223 lines): diff --git a/gdk/gdk_align.c b/gdk/gdk_align.c --- a/gdk/gdk_align.c +++ b/gdk/gdk_align.c @@ -274,114 +274,99 @@ VIEWunlink(BAT *b) gdk_return VIEWreset(BAT *b) { - bat tp, tvp; - Heap tail, *th = NULL; - BAT *v = NULL; - if (b == NULL) return GDK_FAIL; - assert(b->batCacheid > 0); - tp = VIEWtparent(b); - tvp = VIEWvtparent(b); - if (tp || tvp) { - BUN cnt; - const char *nme; - /* alloc heaps */ - tail = (Heap) {0}; - - cnt = BATcount(b) + 1; - nme = BBP_physical(b->batCacheid); - - assert(b->batCacheid > 0); - assert(tp || tvp || !b->ttype); + bat tp = VIEWtparent(b); + bat tvp = VIEWvtparent(b); + if (tp == 0 && tvp == 0) + return GDK_SUCCEED; - tail.farmid = BBPselectfarm(b->batRole, b->ttype, offheap); - strconcat_len(tail.filename, sizeof(tail.filename), - nme, ".tail", NULL); - if (b->ttype && HEAPalloc(&tail, cnt, Tsize(b)) != GDK_SUCCEED) - goto bailout; - if (b->tvheap) { - th = GDKzalloc(sizeof(Heap)); - if (th == NULL) - goto bailout; - th->farmid = BBPselectfarm(b->batRole, b->ttype, varheap); - strconcat_len(th->filename, sizeof(th->filename), - nme, ".theap", NULL); - if (ATOMheap(b->ttype, th, cnt) != GDK_SUCCEED) - goto bailout; - } + if (tp == 0) { + /* only sharing the vheap */ + assert(ATOMstorage(b->ttype) == TYPE_str); + return unshare_string_heap(b); + } - v = VIEWcreate(b->hseqbase, b); - if (v == NULL) - goto bailout; + BAT *v = VIEWcreate(b->hseqbase, b); + if (v == NULL) + return GDK_FAIL; - /* cut the link to your parents */ - VIEWunlink(b); - if (tp) { - BBPunshare(tp); - BBPunfix(tp); - } - if (tvp) { - BBPunshare(tvp); - BBPunfix(tvp); - } - - b->hseqbase = v->hseqbase; + assert(VIEWtparent(v) != b->batCacheid); + assert(VIEWvtparent(v) != b->batCacheid); - b->ttype = v->ttype; - b->tvarsized = v->tvarsized; - b->tshift = v->tshift; - b->twidth = v->twidth; - b->tseqbase = v->tseqbase; - - b->theap.parentid = 0; - b->batRestricted = BAT_WRITE; - - b->tkey = BATtkey(v); + const char *nme = BBP_physical(b->batCacheid); + BUN cnt = BATcount(b); + Heap tail = (Heap) { + .farmid = BBPselectfarm(b->batRole, b->ttype, offheap), + }; + strconcat_len(tail.filename, sizeof(tail.filename), nme, ".tail", NULL); + if (b->ttype && HEAPalloc(&tail, cnt, Tsize(b)) != GDK_SUCCEED) { + BBPunfix(v->batCacheid); + return GDK_FAIL; + } + Heap *th = NULL; + if (b->tvheap) { + th = GDKmalloc(sizeof(Heap)); + if (th == NULL) { + HEAPfree(&tail, true); + BBPunfix(v->batCacheid); + return GDK_FAIL; + } + *th = (Heap) { + .farmid = BBPselectfarm(b->batRole, b->ttype, varheap), + .parentid = b->batCacheid, + }; + strconcat_len(th->filename, sizeof(th->filename), + nme, ".theap", NULL); + if (ATOMheap(b->ttype, th, cnt) != GDK_SUCCEED) { + HEAPfree(&tail, true); + GDKfree(th); + BBPunfix(v->batCacheid); + return GDK_FAIL; + } + } - /* copy the heaps */ - b->theap = tail; - - /* unshare from parents heap */ - if (th) { - assert(b->tvheap == NULL); - b->tvheap = th; - th = NULL; - b->tvheap->parentid = b->batCacheid; - } + BAT bak = *b; /* backup copy */ - if (v->theap.parentid == b->batCacheid) { - assert(tp == 0); - assert(b->batSharecnt > 0); - BBPunshare(b->batCacheid); - BBPunfix(b->batCacheid); - v->theap.parentid = 0; + /* start overwriting */ + VIEWunlink(b); + b->tvheap = th; + b->theap = tail; + /* we empty out b completely and then fill it from v */ + DELTAinit(b); + b->batCapacity = cnt; + b->batCopiedtodisk = false; + b->batDirtydesc = true; + b->batRestricted = BAT_WRITE; + b->tkey = true; + b->tsorted = b->trevsorted = true; + b->tnosorted = b->tnorevsorted = 0; + b->tnokey[0] = b->tnokey[1] = 0; + b->tnil = false; + b->tnonil = true; + if (BATappend2(b, v, NULL, false, false) != GDK_SUCCEED) { + /* clean up the mess */ + if (th) { + HEAPfree(th, true); + GDKfree(th); } - b->batSharecnt = 0; - b->batCopiedtodisk = false; - b->batDirtydesc = true; - - b->tkey = BATtkey(v); - - /* make the BAT empty and insert all again */ - DELTAinit(b); - /* reset capacity */ - b->batCapacity = cnt; - - /* insert all of v in b, and quit */ - if (BATappend2(b, v, NULL, false, false) != GDK_SUCCEED) { - GDKerror("appending view failed"); - goto bailout; - } - BBPreclaim(v); + HEAPfree(&tail, true); + *b = bak; + BBPunfix(v->batCacheid); + return GDK_FAIL; } + /* point of no return */ + if (tp) { + BBPunshare(tp); + BBPunfix(tp); + } + if (tvp) { + BBPunshare(tvp); + BBPunfix(tvp); + } + BBPunfix(v->batCacheid); return GDK_SUCCEED; - bailout: - BBPreclaim(v); - HEAPfree(&tail, false); - GDKfree(th); - return GDK_FAIL; } /* diff --git a/gdk/gdk_bat.c b/gdk/gdk_bat.c --- a/gdk/gdk_bat.c +++ b/gdk/gdk_bat.c @@ -2171,6 +2171,8 @@ BATassertProps(BAT *b) b->tvheap == NULL || (BBPfarms[b->tvheap->farmid].roles & (1 << b->batRole))); + assert(VIEWtparent(b) == 0 || b->tvheap == NULL || b->tvheap->parentid != b->batCacheid); + cmpf = ATOMcompare(b->ttype); nilp = ATOMnilptr(b->ttype); diff --git a/gdk/gdk_batop.c b/gdk/gdk_batop.c --- a/gdk/gdk_batop.c +++ b/gdk/gdk_batop.c @@ -22,7 +22,7 @@ gdk_return unshare_string_heap(BAT *b) { assert(b->batCacheid > 0); - if (b->ttype == TYPE_str && + if (ATOMstorage(b->ttype) == TYPE_str && b->tvheap->parentid != b->batCacheid) { Heap *h = GDKzalloc(sizeof(Heap)); if (h == NULL) _______________________________________________ checkin-list mailing list checkin-list@monetdb.org https://www.monetdb.org/mailman/listinfo/checkin-list