You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@parquet.apache.org by bl...@apache.org on 2015/03/05 02:57:01 UTC
incubator-parquet-mr git commit: PARQUET-186: Fix Precondition
performance problem in SnappyUtil.
Repository: incubator-parquet-mr
Updated Branches:
refs/heads/master d084ad29e -> ea81e9aac
PARQUET-186: Fix Precondition performance problem in SnappyUtil.
This fixes the problem by adding string formatting to the preconditions. This avoids any string formatting unless the precondition throws an Exception. We should check for string operations in other tight loops as well.
Author: Ryan Blue <bl...@apache.org>
Closes #133 from rdblue/PARQUET-186-precondition-format-string and squashes the following commits:
be0b8fe [Ryan Blue] PARQUET-186: Fix Precondition performance bug in SnappyUtil.
67f9bf2 [Ryan Blue] PARQUET-186: Add format string and args to Preconditions.
Project: http://git-wip-us.apache.org/repos/asf/incubator-parquet-mr/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-parquet-mr/commit/ea81e9aa
Tree: http://git-wip-us.apache.org/repos/asf/incubator-parquet-mr/tree/ea81e9aa
Diff: http://git-wip-us.apache.org/repos/asf/incubator-parquet-mr/diff/ea81e9aa
Branch: refs/heads/master
Commit: ea81e9aac328b2a89226417e58d4d8366891a9f4
Parents: d084ad2
Author: Ryan Blue <bl...@apache.org>
Authored: Wed Mar 4 17:56:52 2015 -0800
Committer: Ryan Blue <bl...@apache.org>
Committed: Wed Mar 4 17:56:52 2015 -0800
----------------------------------------------------------------------
.../src/main/java/parquet/Preconditions.java | 79 +++++++++++++++++---
.../test/java/parquet/TestPreconditions.java | 58 ++++++++++++++
.../java/parquet/hadoop/codec/SnappyUtil.java | 4 +-
3 files changed, 129 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-parquet-mr/blob/ea81e9aa/parquet-common/src/main/java/parquet/Preconditions.java
----------------------------------------------------------------------
diff --git a/parquet-common/src/main/java/parquet/Preconditions.java b/parquet-common/src/main/java/parquet/Preconditions.java
index 01c1d75..1292b83 100644
--- a/parquet-common/src/main/java/parquet/Preconditions.java
+++ b/parquet-common/src/main/java/parquet/Preconditions.java
@@ -41,24 +41,83 @@ public final class Preconditions {
}
/**
- * @param valid whether the argument is valid
- * @param message error message if the argument is not valid
- * @throws IllegalArgumentException if !valid
+ * Precondition-style validation that throws {@link IllegalArgumentException}.
+ *
+ * @param isValid
+ * {@code true} if valid, {@code false} if an exception should be
+ * thrown
+ * @param message
+ * A String message for the exception.
+ * @throws IllegalArgumentException if {@code isValid} is false
*/
- public static void checkArgument(boolean valid, String message) throws IllegalArgumentException {
- if (!valid) {
+ public static void checkArgument(boolean isValid, String message) throws IllegalArgumentException {
+ if (!isValid) {
throw new IllegalArgumentException(message);
}
}
/**
- * @param valid whether the argument is valid
- * @param message error message if the argument is not valid
- * @throws IllegalStateException if !valid
+ * Precondition-style validation that throws {@link IllegalArgumentException}.
+ *
+ * @param isValid
+ * {@code true} if valid, {@code false} if an exception should be
+ * thrown
+ * @param message
+ * A String message for the exception.
+ * @param args
+ * Objects used to fill in {@code %s} placeholders in the message
+ * @throws IllegalArgumentException if {@code isValid} is false
*/
- public static void checkState(boolean valid, String message) throws IllegalStateException {
- if (!valid) {
+ public static void checkArgument(boolean isValid, String message, Object... args)
+ throws IllegalArgumentException {
+ if (!isValid) {
+ throw new IllegalArgumentException(
+ String.format(String.valueOf(message), strings(args)));
+ }
+ }
+
+ /**
+ * Precondition-style validation that throws {@link IllegalStateException}.
+ *
+ * @param isValid
+ * {@code true} if valid, {@code false} if an exception should be
+ * thrown
+ * @param message
+ * A String message for the exception.
+ * @throws IllegalStateException if {@code isValid} is false
+ */
+ public static void checkState(boolean isValid, String message) throws IllegalStateException {
+ if (!isValid) {
throw new IllegalStateException(message);
}
}
+
+ /**
+ * Precondition-style validation that throws {@link IllegalStateException}.
+ *
+ * @param isValid
+ * {@code true} if valid, {@code false} if an exception should be
+ * thrown
+ * @param message
+ * A String message for the exception.
+ * @param args
+ * Objects used to fill in {@code %s} placeholders in the message
+ * @throws IllegalStateException if {@code isValid} is false
+ */
+ public static void checkState(boolean isValid, String message, Object... args)
+ throws IllegalStateException {
+ if (!isValid) {
+ throw new IllegalStateException(
+ String.format(String.valueOf(message), strings(args)));
+ }
+ }
+
+ private static String[] strings(Object[] objects) {
+ String[] strings = new String[objects.length];
+ for (int i = 0; i < objects.length; i += 1) {
+ strings[i] = String.valueOf(objects[i]);
+ }
+ return strings;
+ }
+
}
http://git-wip-us.apache.org/repos/asf/incubator-parquet-mr/blob/ea81e9aa/parquet-common/src/test/java/parquet/TestPreconditions.java
----------------------------------------------------------------------
diff --git a/parquet-common/src/test/java/parquet/TestPreconditions.java b/parquet-common/src/test/java/parquet/TestPreconditions.java
new file mode 100644
index 0000000..ffcd00f
--- /dev/null
+++ b/parquet-common/src/test/java/parquet/TestPreconditions.java
@@ -0,0 +1,58 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package parquet;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestPreconditions {
+ @Test
+ public void testCheckArgument() {
+ try {
+ Preconditions.checkArgument(true, "Test message: %s %s", 12, null);
+ } catch (IllegalArgumentException e) {
+ Assert.fail("Should not throw exception when isValid is true");
+ }
+
+ try {
+ Preconditions.checkArgument(false, "Test message: %s %s", 12, null);
+ Assert.fail("Should throw exception when isValid is false");
+ } catch (IllegalArgumentException e) {
+ Assert.assertEquals("Should format message",
+ "Test message: 12 null", e.getMessage());
+ }
+ }
+
+ @Test
+ public void testCheckState() {
+ try {
+ Preconditions.checkState(true, "Test message: %s %s", 12, null);
+ } catch (IllegalStateException e) {
+ Assert.fail("Should not throw exception when isValid is true");
+ }
+
+ try {
+ Preconditions.checkState(false, "Test message: %s %s", 12, null);
+ Assert.fail("Should throw exception when isValid is false");
+ } catch (IllegalStateException e) {
+ Assert.assertEquals("Should format message",
+ "Test message: 12 null", e.getMessage());
+ }
+ }
+}
http://git-wip-us.apache.org/repos/asf/incubator-parquet-mr/blob/ea81e9aa/parquet-hadoop/src/main/java/parquet/hadoop/codec/SnappyUtil.java
----------------------------------------------------------------------
diff --git a/parquet-hadoop/src/main/java/parquet/hadoop/codec/SnappyUtil.java b/parquet-hadoop/src/main/java/parquet/hadoop/codec/SnappyUtil.java
index 35e74c0..3c349d0 100644
--- a/parquet-hadoop/src/main/java/parquet/hadoop/codec/SnappyUtil.java
+++ b/parquet-hadoop/src/main/java/parquet/hadoop/codec/SnappyUtil.java
@@ -27,7 +27,7 @@ public class SnappyUtil {
public static void validateBuffer(byte[] buffer, int off, int len) {
Preconditions.checkNotNull(buffer, "buffer");
Preconditions.checkArgument(off >= 0 && len >= 0 && off <= buffer.length - len,
- "Invalid offset or length. Out of buffer bounds. buffer.length=" + buffer.length
- + " off=" + off + " len=" + len);
+ "Invalid buffer offset or length: buffer.length=%s off=%s len=%s",
+ buffer.length, off, len);
}
}