keith-turner closed pull request #366: ACCUMULO-4779 Speedup Property by
precomputing and avoiding sync
URL: https://github.com/apache/accumulo/pull/366
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/core/src/main/java/org/apache/accumulo/core/conf/Property.java
b/core/src/main/java/org/apache/accumulo/core/conf/Property.java
index 800db907b1..32ebb4a1cd 100644
--- a/core/src/main/java/org/apache/accumulo/core/conf/Property.java
+++ b/core/src/main/java/org/apache/accumulo/core/conf/Property.java
@@ -39,6 +39,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.base.Preconditions;
+
public enum Property {
// Crypto-related properties
@Experimental
@@ -589,7 +591,16 @@
;
- private String key, defaultValue, description;
+ private String key;
+ private String defaultValue;
+ private String computedDefaultValue;
+ private String description;
+ private boolean annotationsComputed = false;
+ private boolean defaultValueComputed = false;
+ private boolean isSensitive;
+ private boolean isDeprecated;
+ private boolean isExperimental;
+ private boolean isInterpolated;
private PropertyType type;
private static final Logger log = LoggerFactory.getLogger(Property.class);
@@ -629,6 +640,11 @@ public String getRawDefaultValue() {
* @return default value
*/
public String getDefaultValue() {
+ Preconditions.checkState(defaultValueComputed, "precomputeDefaultValue()
must be called before calling this method");
+ return computedDefaultValue;
+ }
+
+ private void precomputeDefaultValue() {
String v;
if (isInterpolated()) {
PropertiesConfiguration pconf = new PropertiesConfiguration();
@@ -643,7 +659,9 @@ public String getDefaultValue() {
}
if (this.type == PropertyType.ABSOLUTEPATH && !(v.trim().equals("")))
v = new File(v).getAbsolutePath();
- return v;
+
+ computedDefaultValue = v;
+ defaultValueComputed = true;
}
/**
@@ -665,7 +683,8 @@ public String getDescription() {
}
private boolean isInterpolated() {
- return hasAnnotation(Interpolated.class) ||
hasPrefixWithAnnotation(getKey(), Interpolated.class);
+ Preconditions.checkState(annotationsComputed, "precomputeAnnotations()
must be called before calling this method");
+ return isInterpolated;
}
/**
@@ -674,7 +693,8 @@ private boolean isInterpolated() {
* @return true if this property is experimental
*/
public boolean isExperimental() {
- return hasAnnotation(Experimental.class) ||
hasPrefixWithAnnotation(getKey(), Experimental.class);
+ Preconditions.checkState(annotationsComputed, "precomputeAnnotations()
must be called before calling this method");
+ return isExperimental;
}
/**
@@ -683,21 +703,26 @@ public boolean isExperimental() {
* @return true if this property is deprecated
*/
public boolean isDeprecated() {
- return hasAnnotation(Deprecated.class) ||
hasPrefixWithAnnotation(getKey(), Deprecated.class);
+ Preconditions.checkState(annotationsComputed, "precomputeAnnotations()
must be called before calling this method");
+ return isDeprecated;
}
- private volatile Boolean isSensitive = null;
-
/**
* Checks if this property is sensitive.
*
* @return true if this property is sensitive
*/
public boolean isSensitive() {
- if (isSensitive == null) {
- isSensitive = hasAnnotation(Sensitive.class) ||
hasPrefixWithAnnotation(getKey(), Sensitive.class);
- }
- return isSensitive.booleanValue();
+ Preconditions.checkState(annotationsComputed, "precomputeAnnotations()
must be called before calling this method");
+ return isSensitive;
+ }
+
+ private void precomputeAnnotations() {
+ isSensitive = hasAnnotation(Sensitive.class) ||
hasPrefixWithAnnotation(getKey(), Sensitive.class);
+ isDeprecated = hasAnnotation(Deprecated.class) ||
hasPrefixWithAnnotation(getKey(), Deprecated.class);
+ isExperimental = hasAnnotation(Experimental.class) ||
hasPrefixWithAnnotation(getKey(), Experimental.class);
+ isInterpolated = hasAnnotation(Interpolated.class) ||
hasPrefixWithAnnotation(getKey(), Interpolated.class);
+ annotationsComputed = true;
}
/**
@@ -709,7 +734,20 @@ public boolean isSensitive() {
* @return true if property is sensitive
*/
public static boolean isSensitive(String key) {
- return hasPrefixWithAnnotation(key, Sensitive.class);
+ Property prop = propertiesByKey.get(key);
+ if (prop != null) {
+ return prop.isSensitive();
+ } else {
+ for (String prefix : validPrefixes) {
+ if (key.startsWith(prefix)) {
+ if (propertiesByKey.get(prefix).isSensitive()) {
+ return true;
+ }
+ }
+ }
+ }
+
+ return false;
}
private <T extends Annotation> boolean hasAnnotation(Class<T>
annotationType) {
@@ -727,24 +765,21 @@ public static boolean isSensitive(String key) {
}
private static <T extends Annotation> boolean hasPrefixWithAnnotation(String
key, Class<T> annotationType) {
- // relies on side-effects of isValidPropertyKey to populate validPrefixes
- if (isValidPropertyKey(key)) {
- // check if property exists on its own and has the annotation
- if (Property.getPropertyByKey(key) != null)
- return getPropertyByKey(key).hasAnnotation(annotationType);
- // can't find the property, so check the prefixes
- boolean prefixHasAnnotation = false;
- for (String prefix : validPrefixes)
- if (key.startsWith(prefix))
- prefixHasAnnotation = prefixHasAnnotation ||
getPropertyByKey(prefix).hasAnnotation(annotationType);
- return prefixHasAnnotation;
+ for (String prefix : validPrefixes) {
+ if (key.startsWith(prefix)) {
+ if (propertiesByKey.get(prefix).hasAnnotation(annotationType)) {
+ return true;
+ }
+ }
}
+
return false;
}
private static HashSet<String> validTableProperties = null;
private static HashSet<String> validProperties = null;
private static HashSet<String> validPrefixes = null;
+ private static HashMap<String,Property> propertiesByKey = null;
private static boolean isKeyValidlyPrefixed(String key) {
for (String prefix : validPrefixes) {
@@ -763,20 +798,7 @@ private static boolean isKeyValidlyPrefixed(String key) {
* property key
* @return true if key is valid (recognized, or has a recognized prefix)
*/
- public synchronized static boolean isValidPropertyKey(String key) {
- if (validProperties == null) {
- validProperties = new HashSet<>();
- validPrefixes = new HashSet<>();
-
- for (Property p : Property.values()) {
- if (p.getType().equals(PropertyType.PREFIX)) {
- validPrefixes.add(p.getKey());
- } else {
- validProperties.add(p.getKey());
- }
- }
- }
-
+ public static boolean isValidPropertyKey(String key) {
return validProperties.contains(key) || isKeyValidlyPrefixed(key);
}
@@ -789,20 +811,12 @@ public synchronized static boolean
isValidPropertyKey(String key) {
* property key
* @return true if key is valid for a table property (recognized, or has a
recognized prefix)
*/
- public synchronized static boolean isValidTablePropertyKey(String key) {
- if (validTableProperties == null) {
- validTableProperties = new HashSet<>();
- for (Property p : Property.values()) {
- if (!p.getType().equals(PropertyType.PREFIX) &&
p.getKey().startsWith(Property.TABLE_PREFIX.getKey())) {
- validTableProperties.add(p.getKey());
- }
- }
- }
-
- return validTableProperties.contains(key) ||
key.startsWith(Property.TABLE_CONSTRAINT_PREFIX.getKey())
- || key.startsWith(Property.TABLE_ITERATOR_PREFIX.getKey()) ||
key.startsWith(Property.TABLE_LOCALITY_GROUP_PREFIX.getKey())
- || key.startsWith(Property.TABLE_COMPACTION_STRATEGY_PREFIX.getKey())
|| key.startsWith(Property.TABLE_REPLICATION_TARGET.getKey())
- || key.startsWith(Property.TABLE_ARBITRARY_PROP_PREFIX.getKey());
+ public static boolean isValidTablePropertyKey(String key) {
+ return validTableProperties.contains(key)
+ || (key.startsWith(Property.TABLE_PREFIX.getKey()) &&
(key.startsWith(Property.TABLE_CONSTRAINT_PREFIX.getKey())
+ || key.startsWith(Property.TABLE_ITERATOR_PREFIX.getKey()) ||
key.startsWith(Property.TABLE_LOCALITY_GROUP_PREFIX.getKey())
+ ||
key.startsWith(Property.TABLE_COMPACTION_STRATEGY_PREFIX.getKey()) ||
key.startsWith(Property.TABLE_REPLICATION_TARGET.getKey()) || key
+ .startsWith(Property.TABLE_ARBITRARY_PROP_PREFIX.getKey())));
}
/**
@@ -838,7 +852,7 @@ public static boolean isValidZooPropertyKey(String key) {
return key.startsWith(Property.TABLE_PREFIX.getKey()) ||
key.startsWith(Property.TSERV_PREFIX.getKey()) ||
key.startsWith(Property.LOGGER_PREFIX.getKey())
|| key.startsWith(Property.MASTER_PREFIX.getKey()) ||
key.startsWith(Property.GC_PREFIX.getKey())
|| key.startsWith(Property.MONITOR_PREFIX.getKey() + "banner.") ||
key.startsWith(VFS_CONTEXT_CLASSPATH_PROPERTY.getKey())
- || key.startsWith(Property.TABLE_COMPACTION_STRATEGY_PREFIX.getKey())
|| key.startsWith(REPLICATION_PREFIX.getKey());
+ || key.startsWith(REPLICATION_PREFIX.getKey());
}
/**
@@ -849,10 +863,7 @@ public static boolean isValidZooPropertyKey(String key) {
* @return property, or null if not found
*/
public static Property getPropertyByKey(String key) {
- for (Property prop : Property.values())
- if (prop.getKey().equals(key))
- return prop;
- return null;
+ return propertiesByKey.get(key);
}
/**
@@ -952,4 +963,35 @@ public static boolean isClassProperty(String key) {
}
return result;
}
+
+ static {
+ // Precomputing information here avoids :
+ // * Computing it each time a method is called
+ // * Using synch to compute the first time a method is called
+ propertiesByKey = new HashMap<>();
+ validPrefixes = new HashSet<>();
+ validProperties = new HashSet<>();
+
+ for (Property p : Property.values()) {
+ if (p.getType().equals(PropertyType.PREFIX)) {
+ validPrefixes.add(p.getKey());
+ } else {
+ validProperties.add(p.getKey());
+ }
+ propertiesByKey.put(p.getKey(), p);
+ }
+
+ validTableProperties = new HashSet<>();
+ for (Property p : Property.values()) {
+ if (!p.getType().equals(PropertyType.PREFIX) &&
p.getKey().startsWith(Property.TABLE_PREFIX.getKey())) {
+ validTableProperties.add(p.getKey());
+ }
+ }
+
+ // order is very important here the following code relies on the maps and
sets populated above
+ for (Property p : Property.values()) {
+ p.precomputeAnnotations();
+ p.precomputeDefaultValue();
+ }
+ }
}
diff --git a/core/src/test/java/org/apache/accumulo/core/conf/PropertyTest.java
b/core/src/test/java/org/apache/accumulo/core/conf/PropertyTest.java
index 6848ee4070..d25c8c2ffc 100644
--- a/core/src/test/java/org/apache/accumulo/core/conf/PropertyTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/conf/PropertyTest.java
@@ -19,6 +19,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import java.io.File;
@@ -152,4 +153,61 @@ public void validatePropertyKeys() {
public void testGCDeadServerWaitSecond() {
assertEquals("1h", Property.GC_WAL_DEAD_SERVER_WAIT.getDefaultValue());
}
+
+ @SuppressWarnings("deprecation")
+ private Property getDeprecatedProperty() {
+ return Property.INSTANCE_DFS_DIR;
+ }
+
+ @Test
+ public void testAnnotations() {
+ assertTrue(Property.TABLE_VOLUME_CHOOSER.isExperimental());
+ assertFalse(Property.LOGGER_DIR.isExperimental());
+
+ assertTrue(Property.INSTANCE_SECRET.isSensitive());
+ assertFalse(Property.INSTANCE_VOLUMES.isSensitive());
+
+ assertTrue(getDeprecatedProperty().isDeprecated());
+ assertFalse(Property.INSTANCE_VOLUMES_REPLACEMENTS.isDeprecated());
+
+ }
+
+ @Test
+ public void testGetPropertyByKey() {
+ for (Property prop : Property.values()) {
+ assertSame(prop, Property.getPropertyByKey(prop.getKey()));
+ }
+ }
+
+ @Test
+ public void testIsValidPropertyKey() {
+ for (Property prop : Property.values()) {
+ assertTrue(Property.isValidPropertyKey(prop.getKey()));
+ if (prop.getType().equals(PropertyType.PREFIX)) {
+ assertTrue(Property.isValidPropertyKey(prop.getKey() + "foo9"));
+ }
+ }
+
+ assertFalse(Property.isValidPropertyKey("abc.def"));
+ }
+
+ @Test
+ public void testIsValidTablePropertyKey() {
+ for (Property prop : Property.values()) {
+ if (prop.getKey().startsWith("table.") &&
!prop.getKey().equals("table.")) {
+ assertTrue(Property.isValidTablePropertyKey(prop.getKey()));
+
+ if (prop.getType().equals(PropertyType.PREFIX)) {
+ assertTrue(Property.isValidTablePropertyKey(prop.getKey() + "foo9"));
+ } else {
+ assertFalse(Property.isValidTablePropertyKey(prop.getKey() +
"foo9"));
+ }
+ } else {
+ assertFalse(Property.isValidTablePropertyKey(prop.getKey()));
+ }
+
+ }
+
+ assertFalse(Property.isValidTablePropertyKey("abc.def"));
+ }
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
