From 666cfe87d239780f455db9a4f1a3ff8557d7da0a Mon Sep 17 00:00:00 2001 From: Peter Slawski Date: Tue, 5 Apr 2016 14:21:55 -0700 Subject: [PATCH] HIVE-13699: Make JavaDataModel#get thread safe for parallel compilation --- storage-api/pom.xml | 11 ++++ .../apache/hadoop/hive/ql/util/JavaDataModel.java | 33 ++++++++---- .../hadoop/hive/ql/util/JavaDataModelTest.java | 59 ++++++++++++++++++++++ 3 files changed, 92 insertions(+), 11 deletions(-) create mode 100644 storage-api/src/test/org/apache/hadoop/hive/ql/util/JavaDataModelTest.java diff --git a/storage-api/pom.xml b/storage-api/pom.xml index f5b326b..43815c2 100644 --- a/storage-api/pom.xml +++ b/storage-api/pom.xml @@ -32,6 +32,17 @@ + + + org.apache.logging.log4j + log4j-1.2-api + ${log4j2.version} + + + org.apache.logging.log4j + log4j-slf4j-impl + ${log4j2.version} + org.apache.hadoop diff --git a/storage-api/src/java/org/apache/hadoop/hive/ql/util/JavaDataModel.java b/storage-api/src/java/org/apache/hadoop/hive/ql/util/JavaDataModel.java index 151f30d..dfdde24 100644 --- a/storage-api/src/java/org/apache/hadoop/hive/ql/util/JavaDataModel.java +++ b/storage-api/src/java/org/apache/hadoop/hive/ql/util/JavaDataModel.java @@ -18,6 +18,10 @@ package org.apache.hadoop.hive.ql.util; +import com.google.common.annotations.VisibleForTesting; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + /** * Estimation of memory footprint of object */ @@ -229,6 +233,8 @@ public static int alignUp(int value, int align) { return (value + align - 1) & ~(align - 1); } + private static final Log LOG = LogFactory.getLog(JavaDataModel.class); + public static final int JAVA32_META = 12; public static final int JAVA32_ARRAY_META = 16; public static final int JAVA32_REF = 4; @@ -246,22 +252,27 @@ public static int alignUp(int value, int align) { public static final int PRIMITIVE_BYTE = 1; // byte - private static JavaDataModel current; + private static final class LazyHolder { + private static final JavaDataModel MODEL_FOR_SYSTEM = getModelForSystem(); + } - public static JavaDataModel get() { - if (current != null) { - return current; - } + @VisibleForTesting + static JavaDataModel getModelForSystem() { + String props = null; try { - String props = System.getProperty("sun.arch.data.model"); - if ("32".equals(props)) { - return current = JAVA32; - } + props = System.getProperty("sun.arch.data.model"); } catch (Exception e) { - // ignore + LOG.warn("Failed to determine java data model, defaulting to 64", e); + } + if ("32".equals(props)) { + return JAVA32; } // TODO: separate model is needed for compressedOops, which can be guessed from memory size. - return current = JAVA64; + return JAVA64; + } + + public static JavaDataModel get() { + return LazyHolder.MODEL_FOR_SYSTEM; } public static int round(int size) { diff --git a/storage-api/src/test/org/apache/hadoop/hive/ql/util/JavaDataModelTest.java b/storage-api/src/test/org/apache/hadoop/hive/ql/util/JavaDataModelTest.java new file mode 100644 index 0000000..35976cc --- /dev/null +++ b/storage-api/src/test/org/apache/hadoop/hive/ql/util/JavaDataModelTest.java @@ -0,0 +1,59 @@ +package org.apache.hadoop.hive.ql.util; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertSame; + +public final class JavaDataModelTest { + + private static final String DATA_MODEL_PROPERTY = "sun.arch.data.model"; + + private String previousModelSetting; + + @Before + public void setUp() throws Exception { + previousModelSetting = System.getProperty(DATA_MODEL_PROPERTY); + } + + @After + public void tearDown() throws Exception { + if (previousModelSetting != null) { + System.setProperty(DATA_MODEL_PROPERTY, previousModelSetting); + } else { + System.clearProperty(DATA_MODEL_PROPERTY); + } + } + + @Test + public void testGetDoesNotReturnNull() throws Exception { + JavaDataModel model = JavaDataModel.get(); + assertNotNull(model); + } + + @Test + public void testGetModelForSystemWhenSetTo32() throws Exception { + System.setProperty(DATA_MODEL_PROPERTY, "32"); + assertSame(JavaDataModel.JAVA32, JavaDataModel.getModelForSystem()); + } + + @Test + public void testGetModelForSystemWhenSetTo64() throws Exception { + System.setProperty(DATA_MODEL_PROPERTY, "64"); + assertSame(JavaDataModel.JAVA64, JavaDataModel.getModelForSystem()); + } + + @Test + public void testGetModelForSystemWhenSetToUnknown() throws Exception { + System.setProperty(DATA_MODEL_PROPERTY, "unknown"); + assertSame(JavaDataModel.JAVA64, JavaDataModel.getModelForSystem()); + } + + @Test + public void testGetModelForSystemWhenUndefined() throws Exception { + System.clearProperty(DATA_MODEL_PROPERTY); + assertSame(JavaDataModel.JAVA64, JavaDataModel.getModelForSystem()); + } +} \ No newline at end of file -- 2.5.4 (Apple Git-61)