From e97fc7f176f8bf501c9b3afd8410014e3b0e1602 Mon Sep 17 00:00:00 2001 From: Sean Owen Date: Thu, 3 Mar 2016 09:54:09 +0000 Subject: [SPARK-13423][WIP][CORE][SQL][STREAMING] Static analysis fixes for 2.x ## What changes were proposed in this pull request? Make some cross-cutting code improvements according to static analysis. These are individually up for discussion since they exist in separate commits that can be reverted. The changes are broadly: - Inner class should be static - Mismatched hashCode/equals - Overflow in compareTo - Unchecked warnings - Misuse of assert, vs junit.assert - get(a) + getOrElse(b) -> getOrElse(a,b) - Array/String .size -> .length (occasionally, -> .isEmpty / .nonEmpty) to avoid implicit conversions - Dead code - tailrec - exists(_ == ) -> contains find + nonEmpty -> exists filter + size -> count - reduce(_+_) -> sum map + flatten -> map The most controversial may be .size -> .length simply because of its size. It is intended to avoid implicits that might be expensive in some places. ## How was the this patch tested? Existing Jenkins unit tests. Author: Sean Owen Closes #11292 from srowen/SPARK-13423. --- .../spark/network/shuffle/protocol/mesos/RegisterDriver.java | 8 ++++++++ .../spark/network/shuffle/ExternalShuffleIntegrationSuite.java | 2 +- .../apache/spark/network/shuffle/RetryingBlockFetcherSuite.java | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) (limited to 'common/network-shuffle') diff --git a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/protocol/mesos/RegisterDriver.java b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/protocol/mesos/RegisterDriver.java index 94a61d6caa..eeb0019411 100644 --- a/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/protocol/mesos/RegisterDriver.java +++ b/common/network-shuffle/src/main/java/org/apache/spark/network/shuffle/protocol/mesos/RegisterDriver.java @@ -56,6 +56,14 @@ public class RegisterDriver extends BlockTransferMessage { return Objects.hashCode(appId); } + @Override + public boolean equals(Object o) { + if (!(o instanceof RegisterDriver)) { + return false; + } + return Objects.equal(appId, ((RegisterDriver) o).appId); + } + public static RegisterDriver decode(ByteBuf buf) { String appId = Encoders.Strings.decode(buf); return new RegisterDriver(appId); diff --git a/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/ExternalShuffleIntegrationSuite.java b/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/ExternalShuffleIntegrationSuite.java index 5e706bf401..ecbbe7bfa3 100644 --- a/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/ExternalShuffleIntegrationSuite.java +++ b/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/ExternalShuffleIntegrationSuite.java @@ -109,7 +109,7 @@ public class ExternalShuffleIntegrationSuite { handler.applicationRemoved(APP_ID, false /* cleanupLocalDirs */); } - class FetchResult { + static class FetchResult { public Set successBlocks; public Set failedBlocks; public List buffers; diff --git a/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/RetryingBlockFetcherSuite.java b/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/RetryingBlockFetcherSuite.java index 3a6ef0d3f8..91882e3b3b 100644 --- a/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/RetryingBlockFetcherSuite.java +++ b/common/network-shuffle/src/test/java/org/apache/spark/network/shuffle/RetryingBlockFetcherSuite.java @@ -305,7 +305,7 @@ public class RetryingBlockFetcherSuite { } } - assert stub != null; + assertNotNull(stub); stub.when(fetchStarter).createAndStart((String[]) any(), (BlockFetchingListener) anyObject()); String[] blockIdArray = blockIds.toArray(new String[blockIds.size()]); new RetryingBlockFetcher(conf, fetchStarter, blockIdArray, listener).start(); -- cgit v1.2.3