Skip to content

Commit

Permalink
refactor!: Remove Assert/Require methods that require locks/reflections
Browse files Browse the repository at this point in the history
  • Loading branch information
niloc132 committed Jul 17, 2024
1 parent 8f54141 commit 8541504
Show file tree
Hide file tree
Showing 13 changed files with 28 additions and 67 deletions.
42 changes: 0 additions & 42 deletions Base/src/main/java/io/deephaven/base/verify/Assert.java
Original file line number Diff line number Diff line change
Expand Up @@ -316,48 +316,6 @@ public static AssertionFailure valueNeverOccurs(double d, String name) {
return null;
}

// ################################################################
// holdsLock, notHoldsLock

// ----------------------------------------------------------------
/** assert (o != null && (current thread holds o's lock)) */
public static void holdsLock(Object o, String name) {
neqNull(o, "o");
if (!Thread.holdsLock(o)) {
fail("\"" + Thread.currentThread().getName() + "\".holdsLock(" + name + ")");
}
}

// ----------------------------------------------------------------
/** assert (o != null && !(current thread holds o's lock)) */
public static void notHoldsLock(Object o, String name) {
neqNull(o, "o");
if (Thread.holdsLock(o)) {
fail("!\"" + Thread.currentThread().getName() + "\".holdsLock(" + name + ")");
}
}

// ################################################################
// instanceOf, notInstanceOf

// ----------------------------------------------------------------
/** assert (o instanceof type) */
public static void instanceOf(Object o, String name, Class<?> type) {
if (!type.isInstance(o)) {
fail(name + " instanceof " + type, null == o ? ExceptionMessageUtil.valueAndName(o, name)
: name + " instanceof " + o.getClass() + " (" + ExceptionMessageUtil.valueAndName(o, name) + ")");
}
}

// ----------------------------------------------------------------
/** assert !(o instanceof type) */
public static void notInstanceOf(Object o, String name, Class<?> type) {
if (type.isInstance(o)) {
fail("!(" + name + " instanceof " + type + ")",
name + " instanceof " + o.getClass() + " (" + ExceptionMessageUtil.valueAndName(o, name) + ")");
}
}

// ################################################################
// eq (primitiveValue == primitiveValue)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ private List<SERIES> getSeries() {
@Override
// this should be run under a seriesLock
public void addSeries(SERIES series, Object key) {
Assert.holdsLock(seriesLock, "seriesLock");
Assert.assertion(Thread.holdsLock(seriesLock), "Thread.holdsLock(seriesLock)");

if (seriesKeys == null) {
seriesKeys = new HashSet<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ private synchronized void returnPoolableChannel(@NotNull final CachedChannel cac
}

private long advanceClock() {
Assert.holdsLock(this, "this");
Assert.assertion(Thread.holdsLock(this), "Thread.holdsLock(this)");
final long newClock = ++logicalClock;
if (newClock > 0) {
return newClock;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ private boolean isActive() {
* Activate this subscription entry. Must hold the lock on the enclosing subscription set.
*/
public void activate() {
Assert.holdsLock(SubscriptionSet.this, "SubscriptionSet.this");
Assert.assertion(Thread.holdsLock(SubscriptionSet.this), "Thread.holdsLock(SubscriptionSet.this)");
active = true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ public static LongUnaryOperator getReverseLookup(@NotNull final Table parent, @N
Assert.neqNull(value, "sort result reverse lookup");
}
if (value != null) {
Assert.instanceOf(value, "sort result reverse lookup", LongUnaryOperator.class);
Assert.assertion(value instanceof LongUnaryOperator, "sort result reverse lookup");
return (LongUnaryOperator) value;
}
final RowRedirection sortRedirection = getRowRedirection(sortResult);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2005,7 +2005,7 @@ public void supplyRowLookup(@NotNull final Supplier<AggregationRowLookup> rowLoo
public static AggregationRowLookup getRowLookup(@NotNull final Table aggregationResult) {
final Object value = aggregationResult.getAttribute(AGGREGATION_ROW_LOOKUP_ATTRIBUTE);
Assert.neqNull(value, "aggregation result row lookup");
Assert.instanceOf(value, "aggregation result row lookup", AggregationRowLookup.class);
Assert.assertion(value instanceof AggregationRowLookup, "aggregation result row lookup");
return (AggregationRowLookup) value;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1670,8 +1670,9 @@ public Class<?> visit(UnaryExpr n, VisitArgs printer) {
if (isNonequalOpOverload && printer.hasStringBuilder()) {
// sanity checks -- the inner expression *must* be a BinaryExpr (for ==), and it must be replaced in
// this UnaryExpr with a MethodCallExpr (for "eq()" or possibly "isNull()").
Assert.instanceOf(n.getExpression(), "n.getExpression()", MethodCallExpr.class);
final MethodCallExpr methodCall = (MethodCallExpr) n.getExpression();
Object o = n.getExpression();
Assert.assertion(o instanceof MethodCallExpr, "n.getExpression()");
final MethodCallExpr methodCall = (MethodCallExpr) o;
final String methodName = methodCall.getNameAsString();
if (!"eq".equals(methodName) && !"isNull".equals(methodName)) {
throw new IllegalStateException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ private final class PendingChange {
String error;

private PendingChange(@NotNull Table table, boolean delete) {
Assert.holdsLock(pendingChanges, "pendingChanges");
Assert.assertion(Thread.holdsLock(pendingChanges), "Thread.holdsLock(pendingChanges)");
Assert.neqNull(table, "table");
this.table = table;
this.delete = delete;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3921,7 +3921,7 @@ public void testKeyColumnMissing() {
final Table agg = data.selectDistinct("NonExistentCol");
fail("Should have thrown an exception");
} catch (Exception ex) {
io.deephaven.base.verify.Assert.instanceOf(ex, "ex", IllegalArgumentException.class);
assertTrue(ex instanceof IllegalArgumentException);
io.deephaven.base.verify.Assert.assertion(
ex.getMessage().contains("Missing columns: [NonExistentCol]"),
"ex.getMessage().contains(\"Missing columns: [NonExistentCol]\")",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import java.util.concurrent.ExecutionException;
import java.util.concurrent.atomic.AtomicInteger;

import static org.junit.Assert.assertNotNull;

public final class TestJobScheduler {

@Rule
Expand Down Expand Up @@ -149,7 +151,7 @@ public void close() {
50,
(context, idx, nec, resume) -> {
// verify the type is correct
Assert.instanceOf(context, "context", TestJobThreadContext.class);
assertNotNull(context);

completed[idx] = true;
resume.run();
Expand Down Expand Up @@ -262,7 +264,7 @@ public void close() {
50,
(context, idx, nec, resume) -> {
// verify the type is correct
Assert.instanceOf(context, "context", TestJobThreadContext.class);
assertNotNull(context);

completed[idx] = true;
resume.run();
Expand Down Expand Up @@ -396,7 +398,7 @@ public void close() {
50,
(context, idx, nec) -> {
// verify the type is correct
Assert.instanceOf(context, "context", TestJobThreadContext.class);
assertNotNull(context);

// throw before "doing work" to make verification easy
if (idx == 10) {
Expand Down Expand Up @@ -467,7 +469,7 @@ public void close() {
50,
(context, idx, nec, resume) -> {
// verify the type is correct
Assert.instanceOf(context, "context", TestJobThreadContext.class);
assertNotNull(context);

completed[idx] = true;

Expand Down Expand Up @@ -552,7 +554,7 @@ public void close() {
60,
(context2, idx2, nec2) -> {
// verify the type is correct
Assert.instanceOf(context2, "context2", TestJobThreadContext.class);
assertNotNull(context2);

// throw before "doing work" to make verification easy
if (idx1 == 10 && idx2 == 10) {
Expand Down Expand Up @@ -632,7 +634,7 @@ public void close() {
60,
(context2, idx2, nec2) -> {
// verify the type is correct
Assert.instanceOf(context2, "context2", TestJobThreadContext.class);
assertNotNull(context2);
completed[idx1][idx2] = true;
}, r1, nec1);
},
Expand Down Expand Up @@ -702,7 +704,7 @@ public void close() {
60,
(context2, idx2, nec2) -> {
// verify the type is correct
Assert.instanceOf(context2, "context2", TestJobThreadContext.class);
assertNotNull(context2);
completed[idx1][idx2] = true;
}, r1, nec1);
},
Expand Down Expand Up @@ -770,7 +772,7 @@ public void close() {
50,
(context, idx, nec) -> {
// verify the type is correct
Assert.instanceOf(context, "context", TestJobThreadContext.class);
assertNotNull(context);

// throw before "doing work" to make verification easy
if (idx == 10) {
Expand Down Expand Up @@ -825,7 +827,7 @@ public void close() {
50,
(context, idx, nec) -> {
// verify the type is correct
Assert.instanceOf(context, "context", TestJobThreadContext.class);
assertNotNull(context);
completed[idx] = true;
},
() -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ protected synchronized void updateServerViewport(
final RowSet viewport,
final BitSet columns,
final boolean reverseViewport) {
Assert.holdsLock(this, "BarrageTable.this");
Assert.assertion(Thread.holdsLock(this), "Thread.holdsLock(this)");

final RowSet finalViewport = viewport == null ? null : viewport.copy();
final BitSet finalColumns = (columns == null || columns.cardinality() == numColumns())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ public void close() {
}

private void enqueueUpdate(final TableUpdate upstream) {
Assert.holdsLock(this, "enqueueUpdate must hold lock!");
Assert.assertion(Thread.holdsLock(this), "Thread.holdsLock(this)");

final WritableRowSet addsToRecord;
final RowSet modsToRecord;
Expand Down Expand Up @@ -952,7 +952,7 @@ private void enqueueUpdate(final TableUpdate upstream) {
}

private void schedulePropagation() {
Assert.holdsLock(this, "schedulePropagation must hold lock!");
Assert.assertion(Thread.holdsLock(this), "schedulePropagation must hold lock!");

// copy lastUpdateTime so we are not duped by the re-read
final long localLastUpdateTime = lastUpdateTime;
Expand Down Expand Up @@ -1625,7 +1625,7 @@ private void propagateSnapshotForSubscription(final Subscription subscription,
}

private BarrageMessage aggregateUpdatesInRange(final int startDelta, final int endDelta) {
Assert.holdsLock(this, "propagateUpdatesInRange must hold lock!");
Assert.assertion(Thread.holdsLock(this), "propagateUpdatesInRange must hold lock!");

final boolean singleDelta = endDelta - startDelta == 1;
final BarrageMessage downstream = new BarrageMessage();
Expand Down Expand Up @@ -2110,7 +2110,7 @@ private void buildPostSnapshotViewports(boolean ignorePending) {
}

private void promoteSnapshotToActive() {
Assert.holdsLock(this, "promoteSnapshotToActive must hold lock!");
Assert.assertion(Thread.holdsLock(this), "Thread.holdsLock(this)");

if (activeViewport != null) {
activeViewport.close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ public void setViewport(
}

private void scheduleImmediately(final long currentTimeNanos) {
Assert.holdsLock(schedulingLock, "schedulingLock");
Assert.assertion(Thread.holdsLock(schedulingLock), "Thread.holdsLock(schedulingLock)");
if (!snapshotPending || currentTimeNanos < scheduledTimeNanos) {
snapshotPending = true;
scheduledTimeNanos = currentTimeNanos;
Expand All @@ -431,7 +431,7 @@ private void scheduleImmediately(final long currentTimeNanos) {
}

private void scheduleAtInterval(final long currentTimeNanos) {
Assert.holdsLock(schedulingLock, "schedulingLock");
Assert.assertion(Thread.holdsLock(schedulingLock), "Thread.holdsLock(schedulingLock)");
final long targetTimeNanos = lastSnapshotTimeNanos + intervalDurationNanos;
final long delayNanos = targetTimeNanos - currentTimeNanos;
if (delayNanos < 0) {
Expand Down

0 comments on commit 8541504

Please sign in to comment.