Changeset: 142086e32817 for MonetDB
URL: http://dev.monetdb.org/hg/MonetDB?cmd=changeset;node=142086e32817
Modified Files:
        MonetDB/src/gdk/gdk.mx
        MonetDB/src/gdk/gdk_align.mx
        MonetDB/src/gdk/gdk_bbp.mx
        MonetDB/src/gdk/gdk_heap.mx
        MonetDB5/src/modules/kernel/bat5.mx
        MonetDB5/src/modules/kernel/batcalc.mx
Branch: default
Log Message:

Make that views on bats don't cause the parent bat to remain loaded.

When a bat has a view, the physical reference of the parent bat is
incremented, causing it to stay in core while the view exists.
However, the parent bat also remains in core when the view has a
physical reference of 0 (but with a logical reference > 0 so that the
view does still exist).

This patch changes that.

When a view's physical reference count goes to zero, the paren't
reference count is decremented.  This means that if there are no other
references to the parent bat, it too may be swapped out.  When the
view's physical reference count goes up to one, the parent's physical
reference count is also incremented, so that it is loaded again if it
had been swapped out.


diffs (truncated from 623 to 300 lines):

diff -r 40b307447188 -r 142086e32817 MonetDB/src/gdk/gdk.mx
--- a/MonetDB/src/gdk/gdk.mx    Mon May 31 10:58:22 2010 +0200
+++ b/MonetDB/src/gdk/gdk.mx    Mon May 31 15:09:31 2010 +0200
@@ -2094,8 +2094,8 @@
 #define BBPname(i)     (BBPcheck((i),"BBPname")?((i) > 0 || 
BBP_logical(i))?BBP_logical(i):BBP_logical(-(i)):"")
 #define BBPvalid(i)    (BBP_logical(i) != NULL && *BBP_logical(i) != '.')
 #define BATgetId(b)    BBPname((b)->batCacheid)
-#define BBPfix(i)      BBPincref(i, FALSE)
-#define BBPunfix(i)    BBPdecref(i, FALSE)
+#define BBPfix(i)      BBPincref((i), FALSE)
+#define BBPunfix(i)    BBPdecref((i), FALSE)
 
 #define BBPRENAME_ALREADY      (-1)
 #define BBPRENAME_ILLEGAL      (-2)
diff -r 40b307447188 -r 142086e32817 MonetDB/src/gdk/gdk_align.mx
--- a/MonetDB/src/gdk/gdk_align.mx      Mon May 31 10:58:22 2010 +0200
+++ b/MonetDB/src/gdk/gdk_align.mx      Mon May 31 15:09:31 2010 +0200
@@ -560,7 +560,7 @@
        tp = VIEWtparent(b);
        if (hp || tp) {
                BAT *r = b;
-               BAT *n = b = BBP_cache(ABS(b->batCacheid));
+               BAT *n = b = BATdescriptor(ABS(b->batCacheid));
                BAT *m = BATmirror(n);
                BAT *v;
                BATstore *bs = BBP_desc(b->batCacheid);
@@ -615,10 +615,14 @@
 
                /* cut the link to your parents */
                VIEWunlink(n);
-               if (hp)
+               if (hp) {
                        BBPunshare(hp);
-               if (tp)
+                       BBPunfix(hp);
+               }
+               if (tp) {
                        BBPunshare(tp);
+                       BBPunfix(tp);
+               }
 
                /* make sure everything points there */
                m->U = n->U = &bs->U;
@@ -679,10 +683,13 @@
                /* insert all of v in b, and quit */
                BATins(b, v, FALSE);
                BBPreclaim(v);
+               BBPunfix(b->batCacheid);
                b = r;
        }
        return b;
       bailout:
+       if (b)
+               BBPunfix(b->batCacheid);
        HEAPfree(&head);
        HEAPfree(&tail);
        HEAPfree(&hh);
diff -r 40b307447188 -r 142086e32817 MonetDB/src/gdk/gdk_bbp.mx
--- a/MonetDB/src/gdk/gdk_bbp.mx        Mon May 31 10:58:22 2010 +0200
+++ b/MonetDB/src/gdk/gdk_bbp.mx        Mon May 31 15:09:31 2010 +0200
@@ -183,6 +183,7 @@
 static void BBPuncacheit_(bat bid, int unloaddesc);
 static void BBPinitcache(void);
 static int BBPprepare(bit subcommit);
+static BAT *getBBPdescriptor(bat i, int lock);
 
 
 static int stamp = 0;
@@ -934,7 +935,7 @@
 BBPexit(void)
 {
        bat i;
-       int skipped = 1;
+       int skipped;
 
        if (!BBP)
                return;         /* AARGH */
@@ -943,7 +944,7 @@
        BBPlock("BBPexit");     /* stop all threads ever touching more 
descriptors */
 
        /* free all memory (just for leak-checking in Purify) */
-       while (skipped) {
+       do {
                skipped = 0;
                for (i = 0; i < BBPsize; i++) {
                        if (BBPvalid(i)) {
@@ -958,14 +959,22 @@
                                        if (isVIEW(b)) {
                                                bat hp = VIEWhparent(b), tp = 
VIEWtparent(b);
                                                bat vhp = VIEWvhparent(b), vtp 
= VIEWvtparent(b);
-                                               if (hp)
+                                               if (hp) {
                                                        
BBP_cache(hp)->batSharecnt--;
-                                               if (tp)
+                                                       --BBP_lrefs(hp);
+                                               }
+                                               if (tp) {
                                                        
BBP_cache(tp)->batSharecnt--;
-                                               if (vhp)
+                                                       --BBP_lrefs(tp);
+                                               }
+                                               if (vhp) {
                                                        
BBP_cache(vhp)->batSharecnt--;
-                                               if (vtp)
+                                                       --BBP_lrefs(vhp);
+                                               }
+                                               if (vtp) {
                                                        
BBP_cache(vtp)->batSharecnt--;
+                                                       --BBP_lrefs(vtp);
+                                               }
                                        }
                                        if (isVIEW(b))
                                                VIEWdestroy(b);
@@ -989,7 +998,7 @@
                                GDKfree(BBP[i].bak[0]);
                        BBP[i].bak[0] = NULL;
                }
-       }
+       } while (skipped);
        GDKfree(BBP_hash);
        BBP_hash = 0;
 }
@@ -1785,6 +1794,9 @@
 incref(bat i, int logical, int lock)
 {
        int refs = 0;
+       bat hp = 0, tp = 0, hvp = 0, tvp = 0;
+       BATstore *bs = NULL;
+       BAT *b;
 
        if (i == bat_nil) {
                /* Stefan: May this happen? Or should we better call 
GDKerror(), here? */
@@ -1794,25 +1806,139 @@
        if (i < 0)
                i = -i;
        if (BBPcheck(i, "BBPincref")) {
-               if (lock) {
-                       gdk_set_lock(GDKswapLock(i), "BBPincref");
-
-                       while (BBP_status(i) & BBPUNSTABLE) {
-                               gdk_unset_lock(GDKswapLock(i), "BBPincref spin 
wait");
+               int locks[5];
+               int x;
+
+               /* In case we're dealing with a view, we need to lock
+                  both the view and its parent(s).  This is
+                  complicated by the fact that we need to lock the
+                  various BATs in the correct order, since otherwise
+                  we may get into a deadlock with BBPlock, and since
+                  BATs share locks.
+
+                  Another complication is that we only really know
+                  whether we're dealing with a view *after* we've
+                  locked it.  I.e., a catch-22 situation.  What we do
+                  to resolve the issue is tentatively look at which
+                  are the parent BATs, and lock all of them, then
+                  check again, and if something changed, unlock
+                  everything and try again.  This process is very
+                  likely to end.  Only a concurrently running thread
+                  that keeps changing the parent(s) of our BAT can
+                  cause this loop to not terminate.
+               */
+
+               for (;;) {
+                       bs = BBP_desc(i);
+                       /* parent BATs are not relevant for logical refs */
+                       hp = logical ? 0 : bs->B.H->heap.parentid;
+                       tp = logical ? 0 : bs->B.T->heap.parentid;
+                       hvp = logical || bs->B.H->vheap == 0 ? 0 : 
bs->B.H->vheap->parentid;
+                       tvp = logical || bs->B.T->vheap == 0 ? 0 : 
bs->B.T->vheap->parentid;
+
+                       if (!lock)
+                               break;
+
+                       /* we're going to collect lock IDs into locks,
+                          BBP_BATMASK+1 cannot be an ID and is larger
+                          than any valid ID (i.e. sorts to the end) */
+                       x = 0;
+                       locks[x++] = i & BBP_BATMASK;
+                       if (hp)
+                               locks[x++] = ABS(hp) & BBP_BATMASK;
+                       if (tp)
+                               locks[x++] = ABS(tp) & BBP_BATMASK;
+                       if (hvp)
+                               locks[x++] = ABS(hvp) & BBP_BATMASK;
+                       if (tvp)
+                               locks[x++] = ABS(tvp) & BBP_BATMASK;
+                       while (x < 5)
+                               locks[x++] = BBP_BATMASK + 1;
+                       /* sort locks and remove duplicates */
+                       for (x = 0; x < 4; x++) {
+                               int y;
+                               for (y = x + 1; y < 5; y++)
+                                       if (locks[x] > locks[y]) {
+                                               int t = locks[x];
+                                               locks[x] = locks[y];
+                                               locks[y] = t;
+                                       } else if (locks[x] == locks[y])
+                                               locks[y] = BBP_BATMASK + 1;
+                       }
+                       /* lock in sorted order */
+                       for (;;) {
+                               for (x = 0; x < 5 && locks[x] <= BBP_BATMASK; 
x++)
+                                       gdk_set_lock(GDKswapLock(locks[x]),
+                                                    "BBPincref view loop");
+                               if (!(BBP_status(i) & BBPUNSTABLE) &&
+                                   !(hp && BBP_status(hp) & BBPUNSTABLE) &&
+                                   !(tp && BBP_status(tp) & BBPUNSTABLE) &&
+                                   !(hvp && BBP_status(hvp) & BBPUNSTABLE) &&
+                                   !(tvp && BBP_status(tvp) & BBPUNSTABLE))
+                                       break;
+                               /* one of the BATs is "unstable", try again */
+                               for (x = 0; x < 5 && locks[x] <= BBP_BATMASK; 
x++)
+                                       gdk_unset_lock(GDKswapLock(locks[x]),
+                                                      "BBPincref view loop");
                                MT_sleep_ms(1);
-                               gdk_set_lock(GDKswapLock(i), "BBPincref spin 
wait");
+                       }
+
+                       /* check again, now that we hold all locks */
+                       bs = BBP_desc(i);
+                       if (logical ||
+                           (hp == bs->B.H->heap.parentid &&
+                            tp == bs->B.T->heap.parentid &&
+                            ((hvp == 0 && bs->B.H->vheap == NULL) ||
+                             hvp == bs->B.H->vheap->parentid) &&
+                            ((tvp == 0 && bs->B.T->vheap == NULL) ||
+                             tvp == bs->B.T->vheap->parentid)))
+                               break;  /* still the same, we can do the work */
+
+                       /* try again, unlock the locks */
+                       for (x = 0; x < 5 && locks[x] <= BBP_BATMASK; x++)
+                               gdk_unset_lock(GDKswapLock(locks[x]),
+                                              "BBPincref view loop");
+               }
+
+               /* we have all relevant locks */
+               assert(BBP_refs(i) + BBP_lrefs(i) ||
+                      BBP_status(i) & (BBPDELETED | BBPSWAPPED));
+               if (logical)
+                       refs = ++BBP_lrefs(i);
+               else {
+                       refs = ++BBP_refs(i);
+                       if (refs == 1) {
+                               if (hp) {
+                                       incref(hp, 0, 0);
+                                       b = getBBPdescriptor(hp, 0);
+                                       bs->B.H->heap.base = b->H->heap.base + 
(size_t) bs->B.H->heap.base;
+                               }
+                               if (tp) {
+                                       incref(tp, 0, 0);
+                                       if (bs->B.H != bs->B.T) { /* mirror? */
+                                               b = getBBPdescriptor(tp, 0);
+                                               bs->B.T->heap.base = 
b->H->heap.base + (size_t) bs->B.T->heap.base;
+                                       }
+                               }
+                               if (hvp) {
+                                       incref(hvp, 0, 0);
+                                       b = getBBPdescriptor(hvp, 0);
+                                       bs->B.H->vheap->base = 
b->H->vheap->base + (size_t) bs->B.H->vheap->base;
+                               }
+                               if (tvp) {
+                                       incref(tvp, 0, 0);
+                                       if (bs->B.H != bs->B.T) {  /* mirror? */
+                                               b = getBBPdescriptor(tvp, 0);
+                                               bs->B.T->vheap->base = 
b->H->vheap->base + (size_t) bs->B.T->vheap->base;
+                                       }
+                               }
                        }
                }
-               assert(BBP_refs(i) + BBP_lrefs(i) || BBP_status(i) & 
(BBPDELETED | BBPSWAPPED));
-
-               /* got the lock */
-               if (logical) {
-                       refs = ++BBP_lrefs(i);
-               } else {
-                       refs = ++BBP_refs(i);
+               if (lock) {
+                       for (x = 0; x < 5 && locks[x] <= BBP_BATMASK; x++)
+                               gdk_unset_lock(GDKswapLock(locks[x]),
+                                              "BBPincref view loop");
                }
-               if (lock)
-                       gdk_unset_lock(GDKswapLock(i), "BBPfix");
        }
        return refs;
 }
@@ -1834,8 +1960,10 @@
                parent = -parent;
        if (lock)
                gdk_set_lock(GDKswapLock(parent), "BBPshare");
-       if (++BBP_cache(parent)->batSharecnt == 1)
-               (void) incref(parent, FALSE, 0);
+       (void) incref(parent, TRUE, 0);
+       ++BBP_cache(parent)->batSharecnt;
+       assert(BBP_refs(parent) > 0);
+       (void) incref(parent, FALSE, 0);
        if (lock)
                gdk_unset_lock(GDKswapLock(parent), "BBPshare");
 }
_______________________________________________
Checkin-list mailing list
Checkin-list@monetdb.org
http://mail.monetdb.org/mailman/listinfo/checkin-list

Reply via email to