Commit 21aed89e authored by Michael A. Goulet's avatar Michael A. Goulet :cold_sweat: Committed by Michael A. Goulet
Browse files

Fix unnecessary pinning in `btreefile.LeafPage`

We pin an underlying page N times too many whenever we instantiate a new
LeafPage class, due to the constructor of the BTreeFilePageTuple. What we
want is a tuple that acts like a _weak_ reference to the underlying
page, and only make it a _strong_ reference whenever this tuple escapes
the scope of the btree-internal classes (e.g. in `BTreeFile.addTuple`).

Right now, I just add an `unpin()` call in the constructor of the Tuple,
and then pin it whenever I know that it escapes the scope of the btree
package. Maybe there's a more graceful way of doing it. I have no idea.
parent 2b4d8c1f
No related merge requests found
Showing with 30 additions and 7 deletions
+30 -7
......@@ -112,7 +112,7 @@ public class MaterializeNode extends PlanNode {
if (tup != null) {
if (tup.isDiskBacked()) {
// Make an in-memory version of the tuple we can cache.
Tuple copy = new TupleLiteral(tup);
Tuple copy = TupleLiteral.fromTuple(tup);
tup.unpin();
tup = copy;
}
......
......@@ -58,6 +58,14 @@ public class BTreeFilePageTuple extends PageTuple {
"tupleIndex must be at least 0, got " + tupleIndex);
}
unpin(); // TODO(michael): This is gross. I know.
// What we really want here is a tuple that acts like a "weak reference" to its
// underlying page. This is because we actually instantiate these tuples inside
// the page itself (well, inside of LeafPage), so we end up having the page
// pinned N times more than the actual references that escape outside of the
// internal BT data structures. We will re-pin the tuples in the outward-facing
// methods in `BTreeTupleFile`.
this.tupleIndex = tupleIndex;
}
......
......@@ -260,7 +260,7 @@ public class BTreeFileVerifier {
ArrayList<TupleLiteral> keys = new ArrayList<>(numKeys);
if (numKeys > 1) {
Tuple prevKey = inner.getKey(0);
keys.add(new TupleLiteral(prevKey));
keys.add(TupleLiteral.fromTuple(prevKey));
if (parentLeftKey != null) {
int cmp = TupleComparator.compareTuples(parentLeftKey, prevKey);
......@@ -275,7 +275,7 @@ public class BTreeFileVerifier {
for (int k = 1; k < numKeys; k++) {
Tuple key = inner.getKey(k);
keys.add(new TupleLiteral(key));
keys.add(TupleLiteral.fromTuple(key));
int cmp = TupleComparator.compareTuples(prevKey, key);
if (cmp == 0) {
......
......@@ -190,6 +190,7 @@ public class BTreeTupleFile implements SequentialTupleFile {
if (leaf != null && leaf.getNumTuples() > 0)
tup = leaf.getTuple(0);
tup.pin();
return tup;
}
......@@ -256,10 +257,16 @@ public class BTreeTupleFile implements SequentialTupleFile {
logger.error(String.format(
"Next leaf node %d has no entries?!", nextPageNo));
}
dbPage.unpin();
}
}
}
if (nextTuple != null) {
nextTuple.pin();
}
return nextTuple;
}
......@@ -281,8 +288,11 @@ public class BTreeTupleFile implements SequentialTupleFile {
LeafPage leaf = new LeafPage(dbPage, schema);
for (int i = 0; i < leaf.getNumTuples(); i++) {
BTreeFilePageTuple tup = leaf.getTuple(i);
if (tup.getOffset() == fpOffset)
if (tup.getOffset() == fpOffset) {
tup.pin();
return tup;
}
// Tuple offsets within a page will be monotonically increasing.
if (tup.getOffset() > fpOffset)
......@@ -320,6 +330,7 @@ public class BTreeTupleFile implements SequentialTupleFile {
if (cmp == 0) {
// Found it!
tup.pin();
return tup;
}
else if (cmp > 0) {
......@@ -372,8 +383,10 @@ public class BTreeTupleFile implements SequentialTupleFile {
for (int i = 0; i < leaf.getNumTuples(); i++) {
BTreeFilePageTuple tup = leaf.getTuple(i);
int cmp = TupleComparator.comparePartialTuples(tup, searchKey);
if (cmp > 0)
if (cmp > 0) {
tup.pin();
return tup; // Found it!
}
}
leaf.getDBPage().unpin();
......@@ -397,10 +410,12 @@ public class BTreeTupleFile implements SequentialTupleFile {
if (tup instanceof TupleLiteral)
tupLit = (TupleLiteral) tup;
else
tupLit = new TupleLiteral(tup);
tupLit = TupleLiteral.fromTuple(tup);
tupLit.setStorageSize(PageTuple.getTupleStorageSize(schema, tupLit));
return leafPageOps.addTuple(leaf, tupLit, pagePath);
BTreeFilePageTuple bTup = leafPageOps.addTuple(leaf, tupLit, pagePath);
bTup.pin();
return bTup;
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment