Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor!: Remove or inline Assert/Require methods so the classes can be shared with JS API #5782

Merged
merged 5 commits into from
Jul 19, 2024
Merged

refactor!: Remove or inline Assert/Require methods so the classes can be shared with JS API #5782

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8541504
refactor!: Remove Assert/Require methods that require locks/reflections
niloc132 Jul 17, 2024
204b9a9
Assert
niloc132 Jul 17, 2024
98b8748
Remove javadoc
niloc132 Jul 17, 2024
39e7568
Review feedback
niloc132 Jul 19, 2024
bfbb4df
Other suggested fixes
niloc132 Jul 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 0 additions & 48 deletions Base/src/main/java/io/deephaven/base/verify/Assert.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,6 @@
* <li>void valuesNeverOccur(value0, name0, value1, name1, ... )
* </ul>
* <ul>
* <li>void holdsLock/notHoldsLock(Object, String name)
* </ul>
* <ul>
* <li>void instanceOf/notInstanceOf(Object, String name, Class type[, int numCallsBelowRequirer])
* </ul>
* <ul>
* <li>void eq/neq(boolean/char/byte/short/int/long/float/double, String name0,
* boolean/char/byte/short/int/long/float/double[, String name1])
* <li>void lt/leq/gt/geq(char/byte/short/int/long/float/double, String name0, char/byte/short/int/long/float/double[,
Expand Down Expand Up @@ -316,48 +310,6 @@ public static AssertionFailure valueNeverOccurs(double d, String name) {
return null;
}

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

// ----------------------------------------------------------------
/** assert (o != null &amp;&amp; (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 &amp;&amp; !(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
49 changes: 0 additions & 49 deletions Base/src/main/java/io/deephaven/base/verify/Require.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,6 @@
* <li>void valuesNeverOccur(value0, name0, value1, name1, ... )
* </ul>
* <ul>
* <li>void holdsLock/notHoldsLock(Object, String name)
* </ul>
* <ul>
* <li>void instanceOf/notInstanceOf(Object, String name, Class type)
* </ul>
* <ul>
* <li>void eq/neq(boolean/char/byte/short/int/long/float/double, String name0,
* boolean/char/byte/short/int/long/float/double[, String name1])
* <li>void lt/leq/gt/geq(char/byte/short/int/long/float/double, String name0, char/byte/short/int/long/float/double[,
Expand Down Expand Up @@ -337,49 +331,6 @@ public static RequirementFailure valueNeverOccurs(double d, String name) {
return null;
}

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

// ----------------------------------------------------------------

public static void holdsLock(Object o, String name) {
neqNull(o, "o");
if (!Thread.holdsLock(o)) {
fail("\"" + Thread.currentThread().getName() + "\".holdsLock(" + name + ")");
}
}

// ----------------------------------------------------------------

public static void notHoldsLock(Object o, String name) {
neqNull(o, "o");
if (Thread.holdsLock(o)) {
fail("!\"" + Thread.currentThread().getName() + "\".holdsLock(" + name + ")");
}
}


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

// ----------------------------------------------------------------

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

// ----------------------------------------------------------------

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

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

Expand Down
2 changes: 1 addition & 1 deletion Plot/src/main/java/io/deephaven/plot/datasets/multiseries/AbstractMultiSeries.java
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion Util/channel/src/main/java/io/deephaven/util/channel/CachedChannelProvider.java
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion Util/src/main/java/io/deephaven/util/Utils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ public static String getSimpleNameFor(@NotNull Class<?> objectClass) {
* require (o instanceof type)
*/
public static <T> T castTo(Object o, String name, Class<T> type) {
io.deephaven.base.verify.Require.instanceOf(o, name, type);
io.deephaven.base.verify.Require.requirement(type.isInstance(o), name);
// noinspection unchecked
return (T) o;
}
Expand Down
2 changes: 1 addition & 1 deletion Util/src/main/java/io/deephaven/util/datastructures/SubscriptionSet.java
View file
Open in desktop
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
1 change: 0 additions & 1 deletion engine/table/src/main/java/io/deephaven/engine/table/impl/SortOperation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,6 @@ 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);
return (LongUnaryOperator) value;
}
final RowRedirection sortRedirection = getRowRedirection(sortResult);
Expand Down
1 change: 0 additions & 1 deletion engine/table/src/main/java/io/deephaven/engine/table/impl/by/AggregationProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2005,7 +2005,6 @@ 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);
return (AggregationRowLookup) value;
}
}
1 change: 0 additions & 1 deletion engine/table/src/main/java/io/deephaven/engine/table/impl/lang/QueryLanguageParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1670,7 +1670,6 @@ 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();
final String methodName = methodCall.getNameAsString();
if (!"eq".equals(methodName) && !"isNull".equals(methodName)) {
Expand Down
2 changes: 1 addition & 1 deletion ...ne/table/src/main/java/io/deephaven/engine/table/impl/util/BaseArrayBackedInputTable.java
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java
View file
Open in desktop
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
29 changes: 11 additions & 18 deletions engine/table/src/test/java/io/deephaven/engine/util/TestJobScheduler.java
View file
Open in desktop
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 @@ -148,8 +150,7 @@ public void close() {
0,
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 @@ -261,8 +262,7 @@ public void close() {
0,
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 @@ -395,8 +395,7 @@ public void close() {
0,
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 @@ -466,8 +465,7 @@ public void close() {
0,
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 @@ -551,8 +549,7 @@ public void close() {
0,
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 @@ -631,8 +628,7 @@ public void close() {
0,
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 @@ -701,8 +697,7 @@ public void close() {
0,
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 @@ -769,8 +764,7 @@ public void close() {
0,
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 @@ -824,8 +818,7 @@ public void close() {
0,
50,
(context, idx, nec) -> {
// verify the type is correct
Assert.instanceOf(context, "context", TestJobThreadContext.class);
assertNotNull(context);
completed[idx] = true;
},
() -> {
Expand Down
2 changes: 1 addition & 1 deletion extensions/barrage/src/main/java/io/deephaven/extensions/barrage/table/BarrageTable.java
View file
Open in desktop
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
8 changes: 4 additions & 4 deletions server/src/main/java/io/deephaven/server/barrage/BarrageMessageProducer.java
View file
Open in desktop
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), "enqueueUpdate must hold lock!");

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), "promoteSnapshotToActive must hold lock!");

if (activeViewport != null) {
activeViewport.close();
Expand Down
4 changes: 2 additions & 2 deletions ...rc/main/java/io/deephaven/server/hierarchicaltable/HierarchicalTableViewSubscription.java
View file
Open in desktop
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
Loading