PHOENIX-1958 Minimize memory allocation on new connection
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/cd81738b Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/cd81738b Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/cd81738b Branch: refs/heads/calcite Commit: cd81738b1fbcb5cf19123b2dca8da31f602b9c64 Parents: c2fee39 Author: James Taylor <jtay...@salesforce.com> Authored: Sat May 9 10:18:57 2015 -0700 Committer: James Taylor <jtay...@salesforce.com> Committed: Sat May 9 10:18:57 2015 -0700 ---------------------------------------------------------------------- .../apache/phoenix/jdbc/PhoenixConnection.java | 41 +++++++++++--------- .../org/apache/phoenix/util/ReadOnlyProps.java | 32 +++++++++++++++ 2 files changed, 54 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/cd81738b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java index c22a7fa..dad60c1 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java @@ -48,7 +48,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.Properties; import java.util.concurrent.Executor; @@ -56,6 +55,8 @@ import javax.annotation.Nullable; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.client.Consistency; +import org.apache.htrace.Sampler; +import org.apache.htrace.TraceScope; import org.apache.phoenix.call.CallRunner; import org.apache.phoenix.exception.SQLExceptionCode; import org.apache.phoenix.exception.SQLExceptionInfo; @@ -95,15 +96,12 @@ import org.apache.phoenix.util.PropertiesUtil; import org.apache.phoenix.util.ReadOnlyProps; import org.apache.phoenix.util.SQLCloseable; import org.apache.phoenix.util.SQLCloseables; -import org.apache.htrace.Sampler; -import org.apache.htrace.TraceScope; import com.google.common.base.Objects; import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap.Builder; import com.google.common.collect.Lists; -import com.google.common.collect.Maps; /** @@ -185,21 +183,9 @@ public class PhoenixConnection implements Connection, org.apache.phoenix.jdbc.Jd if (tenantId != null) { services = services.getChildQueryServices(tenantId.getBytesPtr()); } - // TODO: we could avoid creating another wrapper if the only property - // specified was for the tenant ID - Map<String, String> existingProps = services.getProps().asMap(); - final Map<String, String> tmpAugmentedProps = Maps.newHashMapWithExpectedSize(existingProps.size() + info.size()); - tmpAugmentedProps.putAll(existingProps); - boolean needsDelegate = false; - for (Entry<Object, Object> entry : this.info.entrySet()) { - String key = entry.getKey().toString(); - String value = entry.getValue().toString(); - String oldValue = tmpAugmentedProps.put(key, value); - needsDelegate |= !Objects.equal(oldValue, value); - } - this.services = !needsDelegate ? services : new DelegateConnectionQueryServices(services) { - final ReadOnlyProps augmentedProps = new ReadOnlyProps(tmpAugmentedProps); - + ReadOnlyProps currentProps = services.getProps(); + final ReadOnlyProps augmentedProps = currentProps.addAll(filterKnownNonProperties(this.info)); + this.services = augmentedProps == currentProps ? services : new DelegateConnectionQueryServices(services) { @Override public ReadOnlyProps getProps() { return augmentedProps; @@ -261,6 +247,23 @@ public class PhoenixConnection implements Connection, org.apache.phoenix.jdbc.Jd this.customTracingAnnotations = getImmutableCustomTracingAnnotations(); } + private static Properties filterKnownNonProperties(Properties info) { + Properties prunedProperties = info; + if (info.contains(PhoenixRuntime.CURRENT_SCN_ATTRIB)) { + if (prunedProperties == info) { + prunedProperties = PropertiesUtil.deepCopy(info); + } + prunedProperties.remove(PhoenixRuntime.CURRENT_SCN_ATTRIB); + } + if (info.contains(PhoenixRuntime.TENANT_ID_ATTRIB)) { + if (prunedProperties == info) { + prunedProperties = PropertiesUtil.deepCopy(info); + } + prunedProperties.remove(PhoenixRuntime.TENANT_ID_ATTRIB); + } + return prunedProperties; + } + private ImmutableMap<String, String> getImmutableCustomTracingAnnotations() { Builder<String, String> result = ImmutableMap.builder(); result.putAll(JDBCUtil.getAnnotations(url, info)); http://git-wip-us.apache.org/repos/asf/phoenix/blob/cd81738b/phoenix-core/src/main/java/org/apache/phoenix/util/ReadOnlyProps.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/ReadOnlyProps.java b/phoenix-core/src/main/java/org/apache/phoenix/util/ReadOnlyProps.java index 68b0879..47137ef 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/util/ReadOnlyProps.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/util/ReadOnlyProps.java @@ -23,10 +23,13 @@ import java.util.HashMap; import java.util.Iterator; import java.util.Map; import java.util.Map.Entry; +import java.util.Properties; import java.util.regex.Matcher; import java.util.regex.Pattern; +import com.google.common.base.Objects; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; /** * @@ -61,6 +64,17 @@ public class ReadOnlyProps implements Iterable<Entry<String, String>> { this.props = ImmutableMap.copyOf(props); } + private ReadOnlyProps(ReadOnlyProps defaultProps, Properties overrides) { + Map<String,String> combinedProps = Maps.newHashMapWithExpectedSize(defaultProps.props.size() + overrides.size()); + combinedProps.putAll(defaultProps.props); + for (Entry<Object, Object> entry : overrides.entrySet()) { + String key = entry.getKey().toString(); + String value = entry.getValue().toString(); + combinedProps.put(key, value); + } + this.props = ImmutableMap.copyOf(combinedProps); + } + private static Pattern varPat = Pattern.compile("\\$\\{[^\\}\\$\u0020]+\\}"); private static int MAX_SUBST = 20; @@ -269,4 +283,22 @@ public class ReadOnlyProps implements Iterable<Entry<String, String>> { public boolean isEmpty() { return props.isEmpty(); } + + /** + * Constructs new map only if necessary for adding the override properties. + * @param overrides Map of properties to override current properties. + * @return new ReadOnlyProps if in applying the overrides there are + * modifications to the current underlying Map, otherwise returns this. + */ + public ReadOnlyProps addAll(Properties overrides) { + for (Entry<Object, Object> entry : overrides.entrySet()) { + String key = entry.getKey().toString(); + String value = entry.getValue().toString(); + String oldValue = props.get(key); + if (!Objects.equal(oldValue, value)) { + return new ReadOnlyProps(this, overrides); + } + } + return this; + } }