Changeset: 3964ff1b8b99 for MonetDB
URL: https://dev.monetdb.org/hg/MonetDB/rev/3964ff1b8b99
Modified Files:
gdk/gdk_aggr.c
gdk/gdk_align.c
gdk/gdk_bat.c
gdk/gdk_batop.c
gdk/gdk_bbp.c
gdk/gdk_group.c
gdk/gdk_hash.c
gdk/gdk_select.c
gdk/gdk_unique.c
Branch: Jul2021
Log Message:
Use heap lock to protect extra properties.
Also use index lock instead of heap lock to lock out concurrent scans
in BATordered and BATordered_rev.
And also reduce number of times we acquire a lock by doing multiple
calls to nolock functions inside one critical section.
diffs (truncated from 822 to 300 lines):
diff --git a/gdk/gdk_aggr.c b/gdk/gdk_aggr.c
--- a/gdk/gdk_aggr.c
+++ b/gdk/gdk_aggr.c
@@ -3614,7 +3614,7 @@ void *
BATmin_skipnil(BAT *b, void *aggr, bit skipnil)
{
const ValRecord *prop;
- const void *res;
+ const void *res = NULL;
size_t s;
lng t0 = 0;
@@ -3628,9 +3628,18 @@ BATmin_skipnil(BAT *b, void *aggr, bit s
}
if (BATcount(b) == 0) {
res = ATOMnilptr(b->ttype);
- } else if ((prop = BATgetprop(b, GDK_MIN_VALUE)) != NULL) {
- res = VALptr(prop);
} else {
+ MT_lock_set(&b->theaplock);
+ if ((prop = BATgetprop_nolock(b, GDK_MIN_VALUE)) != NULL)
+ res = VALptr(prop);
+ else if ((prop = BATgetprop_nolock(b, GDK_MIN_POS)) != NULL) {
+ BATiter bi = bat_iterator_nolock(b);
+ if ((prop = BATsetprop_nolock(b, GDK_MIN_VALUE,
b->ttype, BUNtail(bi, prop->val.oval))) != NULL)
+ res = VALptr(prop);
+ }
+ MT_lock_unset(&b->theaplock);
+ }
+ if (res == NULL) {
oid pos;
BAT *pb = NULL;
@@ -3691,11 +3700,12 @@ BATmin_skipnil(BAT *b, void *aggr, bit s
if (is_oid_nil(pos)) {
res = ATOMnilptr(b->ttype);
} else {
- BATiter bi = bat_iterator(b);
+ MT_lock_set(&b->theaplock);
+ BATiter bi = bat_iterator_nolock(b);
res = BUNtail(bi, pos - b->hseqbase);
- BATsetprop(b, GDK_MIN_VALUE, b->ttype, res);
- BATsetprop(b, GDK_MIN_POS, TYPE_oid, &(oid){pos -
b->hseqbase});
- bat_iterator_end(&bi);
+ BATsetprop_nolock(b, GDK_MIN_VALUE, b->ttype, res);
+ BATsetprop_nolock(b, GDK_MIN_POS, TYPE_oid, &(oid){pos
- b->hseqbase});
+ MT_lock_unset(&b->theaplock);
}
}
if (aggr == NULL) {
@@ -3729,7 +3739,7 @@ void *
BATmax_skipnil(BAT *b, void *aggr, bit skipnil)
{
const ValRecord *prop;
- const void *res;
+ const void *res = NULL;
size_t s;
BATiter bi;
lng t0 = 0;
@@ -3742,9 +3752,18 @@ BATmax_skipnil(BAT *b, void *aggr, bit s
}
if (BATcount(b) == 0) {
res = ATOMnilptr(b->ttype);
- } else if ((prop = BATgetprop(b, GDK_MAX_VALUE)) != NULL) {
- res = VALptr(prop);
} else {
+ MT_lock_set(&b->theaplock);
+ if ((prop = BATgetprop_nolock(b, GDK_MAX_VALUE)) != NULL)
+ res = VALptr(prop);
+ else if ((prop = BATgetprop_nolock(b, GDK_MAX_POS)) != NULL) {
+ BATiter bi = bat_iterator_nolock(b);
+ if ((prop = BATsetprop_nolock(b, GDK_MAX_VALUE,
b->ttype, BUNtail(bi, prop->val.oval))) != NULL)
+ res = VALptr(prop);
+ }
+ MT_lock_unset(&b->theaplock);
+ }
+ if (res == NULL) {
oid pos;
BAT *pb = NULL;
@@ -3796,13 +3815,14 @@ BATmax_skipnil(BAT *b, void *aggr, bit s
if (is_oid_nil(pos)) {
res = ATOMnilptr(b->ttype);
} else {
- bi = bat_iterator(b);
+ MT_lock_set(&b->theaplock);
+ bi = bat_iterator_nolock(b);
res = BUNtail(bi, pos - b->hseqbase);
if (b->tnonil) {
- BATsetprop(b, GDK_MAX_VALUE, b->ttype, res);
- BATsetprop(b, GDK_MAX_POS, TYPE_oid, &(oid){pos
- b->hseqbase});
+ BATsetprop_nolock(b, GDK_MAX_VALUE, b->ttype,
res);
+ BATsetprop_nolock(b, GDK_MAX_POS, TYPE_oid,
&(oid){pos - b->hseqbase});
}
- bat_iterator_end(&bi);
+ MT_lock_unset(&b->theaplock);
}
}
if (aggr == NULL) {
diff --git a/gdk/gdk_align.c b/gdk/gdk_align.c
--- a/gdk/gdk_align.c
+++ b/gdk/gdk_align.c
@@ -254,12 +254,12 @@ BATmaterialize(BAT *b)
b->theap = tail;
b->tbaseoff = 0;
b->theap->dirty = true;
+ BATsetprop_nolock(b, GDK_NUNIQUE, TYPE_oid, &(oid){is_oid_nil(t) ? 1 :
b->batCount});
MT_lock_unset(&b->theaplock);
b->ttype = TYPE_oid;
BATsetdims(b);
b->batDirtydesc = true;
BATsetcount(b, b->batCount);
- BATsetprop(b, GDK_NUNIQUE, TYPE_oid, &(oid){is_oid_nil(t) ? 1 :
b->batCount});
return GDK_SUCCEED;
}
diff --git a/gdk/gdk_bat.c b/gdk/gdk_bat.c
--- a/gdk/gdk_bat.c
+++ b/gdk/gdk_bat.c
@@ -661,14 +661,21 @@ BATfree(BAT *b)
if (b->tident && !default_ident(b->tident))
GDKfree(b->tident);
b->tident = BATstring_t;
+ MT_rwlock_rdlock(&b->thashlock);
+ BUN nunique = BUN_NONE, nbucket = BUN_NONE;
if (b->thash && b->thash != (Hash *) 1) {
- BATsetprop(b, GDK_NUNIQUE, TYPE_oid, &(oid){b->thash->nunique});
- BATsetprop(b, GDK_HASH_BUCKETS, TYPE_oid,
&(oid){b->thash->nbucket});
+ nunique = b->thash->nunique;
+ nbucket = b->thash->nbucket;
}
+ MT_rwlock_rdunlock(&b->thashlock);
HASHfree(b);
IMPSfree(b);
OIDXfree(b);
MT_lock_set(&b->theaplock);
+ if (nunique != BUN_NONE) {
+ BATsetprop_nolock(b, GDK_NUNIQUE, TYPE_oid, &(oid){nunique});
+ BATsetprop_nolock(b, GDK_HASH_BUCKETS, TYPE_oid,
&(oid){nbucket});
+ }
if (b->theap) {
assert(ATOMIC_GET(&b->theap->refs) == 1);
assert(b->theap->parentid == b->batCacheid);
@@ -1249,16 +1256,24 @@ BUNdelete(BAT *b, oid o)
val = BUNtail(bi, p);
if (ATOMlinear(b->ttype) &&
ATOMcmp(b->ttype, ATOMnilptr(b->ttype), val) != 0) {
- if ((prop = BATgetprop(b, GDK_MAX_VALUE)) != NULL
- && ATOMcmp(b->ttype, VALptr(prop), val) >= 0) {
- BATrmprop(b, GDK_MAX_VALUE);
- BATrmprop(b, GDK_MAX_POS);
+ MT_lock_set(&b->theaplock);
+ if ((prop = BATgetprop_nolock(b, GDK_MAX_VALUE)) != NULL) {
+ if (ATOMcmp(b->ttype, VALptr(prop), val) >= 0) {
+ BATrmprop_nolock(b, GDK_MAX_VALUE);
+ BATrmprop_nolock(b, GDK_MAX_POS);
+ }
+ } else {
+ BATrmprop_nolock(b, GDK_MAX_POS);
}
- if ((prop = BATgetprop(b, GDK_MIN_VALUE)) != NULL
- && ATOMcmp(b->ttype, VALptr(prop), val) <= 0) {
- BATrmprop(b, GDK_MIN_VALUE);
- BATrmprop(b, GDK_MIN_POS);
+ if ((prop = BATgetprop_nolock(b, GDK_MIN_VALUE)) != NULL) {
+ if (ATOMcmp(b->ttype, VALptr(prop), val) <= 0) {
+ BATrmprop_nolock(b, GDK_MIN_VALUE);
+ BATrmprop_nolock(b, GDK_MIN_POS);
+ }
+ } else {
+ BATrmprop_nolock(b, GDK_MIN_POS);
}
+ MT_lock_unset(&b->theaplock);
}
if (ATOMunfix(b->ttype, val) != GDK_SUCCEED)
return GDK_FAIL;
@@ -1294,6 +1309,7 @@ BUNdelete(BAT *b, oid o)
b->tnorevsorted = 0;
MT_lock_set(&b->theaplock);
b->batCount--;
+ BATrmprop_nolock(b, GDK_UNIQUE_ESTIMATE);
MT_lock_unset(&b->theaplock);
if (b->batCount <= 1) {
/* some trivial properties */
@@ -1307,7 +1323,6 @@ BUNdelete(BAT *b, oid o)
}
IMPSdestroy(b);
OIDXdestroy(b);
- BATrmprop(b, GDK_UNIQUE_ESTIMATE);
return GDK_SUCCEED;
}
@@ -1358,41 +1373,47 @@ BUNinplacemulti(BAT *b, const oid *posit
if (b->ttype != TYPE_void && ATOMlinear(b->ttype)) {
const ValRecord *prop;
- if ((prop = BATgetprop(b, GDK_MAX_VALUE)) != NULL) {
+ MT_lock_set(&b->theaplock);
+ if ((prop = BATgetprop_nolock(b, GDK_MAX_VALUE)) !=
NULL) {
if (ATOMcmp(b->ttype, t, ATOMnilptr(b->ttype))
!= 0 &&
ATOMcmp(b->ttype, VALptr(prop), t) < 0) {
/* new value is larger than previous
* largest */
- BATsetprop(b, GDK_MAX_VALUE, b->ttype,
t);
- BATsetprop(b, GDK_MAX_POS, TYPE_oid,
&(oid){p});
+ BATsetprop_nolock(b, GDK_MAX_VALUE,
b->ttype, t);
+ BATsetprop_nolock(b, GDK_MAX_POS,
TYPE_oid, &(oid){p});
} else if (ATOMcmp(b->ttype, t, val) != 0 &&
ATOMcmp(b->ttype, VALptr(prop), val)
== 0) {
/* old value is equal to largest and
* new value is smaller (see above),
* so we don't know anymore which is
* the largest */
- BATrmprop(b, GDK_MAX_VALUE);
- BATrmprop(b, GDK_MAX_POS);
+ BATrmprop_nolock(b, GDK_MAX_VALUE);
+ BATrmprop_nolock(b, GDK_MAX_POS);
}
+ } else {
+ BATrmprop_nolock(b, GDK_MAX_POS);
}
- if ((prop = BATgetprop(b, GDK_MIN_VALUE)) != NULL) {
+ if ((prop = BATgetprop_nolock(b, GDK_MIN_VALUE)) !=
NULL) {
if (ATOMcmp(b->ttype, t, ATOMnilptr(b->ttype))
!= 0 &&
ATOMcmp(b->ttype, VALptr(prop), t) > 0) {
/* new value is smaller than previous
* smallest */
- BATsetprop(b, GDK_MIN_VALUE, b->ttype,
t);
- BATsetprop(b, GDK_MIN_POS, TYPE_oid,
&(oid){p});
+ BATsetprop_nolock(b, GDK_MIN_VALUE,
b->ttype, t);
+ BATsetprop_nolock(b, GDK_MIN_POS,
TYPE_oid, &(oid){p});
} else if (ATOMcmp(b->ttype, t, val) != 0 &&
ATOMcmp(b->ttype, VALptr(prop), val)
<= 0) {
/* old value is equal to smallest and
* new value is larger (see above), so
* we don't know anymore which is the
* smallest */
- BATrmprop(b, GDK_MIN_VALUE);
- BATrmprop(b, GDK_MIN_POS);
+ BATrmprop_nolock(b, GDK_MIN_VALUE);
+ BATrmprop_nolock(b, GDK_MIN_POS);
}
+ } else {
+ BATrmprop_nolock(b, GDK_MIN_POS);
}
- BATrmprop(b, GDK_UNIQUE_ESTIMATE);
+ BATrmprop_nolock(b, GDK_UNIQUE_ESTIMATE);
+ MT_lock_unset(&b->theaplock);
} else {
PROPdestroy(b);
}
@@ -2354,6 +2375,9 @@ BATassertProps(BAT *b)
int cmp;
const void *prev = NULL, *valp, *nilp;
+ /* do the complete check within a lock */
+ MT_lock_set(&b->theaplock);
+
/* general BAT sanity */
assert(b != NULL);
assert(b->batCacheid > 0);
@@ -2447,14 +2471,11 @@ BATassertProps(BAT *b)
assert(b->tkey);
assert(b->tnonil);
}
+ MT_lock_unset(&b->theaplock);
return;
}
- /* do the rest on a view in case some other thread changes b */
- BAT *v = VIEWcreate(b->hseqbase, b);
- if (v == NULL)
- return;
- b = v;
- BATiter bi = bat_iterator_nolock(b);
+
+ BATiter bi = bat_iterator_nolock(b);
if (BATtdense(b)) {
assert(b->tseqbase + b->batCount <= GDK_oid_max);
@@ -2468,7 +2489,7 @@ BATassertProps(BAT *b)
for (p = 1; p < q; p++)
assert(o[p - 1] + 1 == o[p]);
}
- BBPunfix(b->batCacheid);
+ MT_lock_unset(&b->theaplock);
return;
}
assert(1 << b->tshift == b->twidth);
@@ -2513,7 +2534,7 @@ BATassertProps(BAT *b)
if (!b->tkey && !b->tsorted && !b->trevsorted &&
!b->tnonil && !b->tnil) {
/* nothing more to prove */
- BBPunfix(b->batCacheid);
+ MT_lock_unset(&b->theaplock);
return;
}
@@ -2524,11 +2545,11 @@ BATassertProps(BAT *b)
bool seenmax = false, seenmin = false;
bool seennil = false;
_______________________________________________
checkin-list mailing list
checkin-list@monetdb.org
https://www.monetdb.org/mailman/listinfo/checkin-list
