You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kylin.apache.org by xx...@apache.org on 2022/10/13 06:46:34 UTC
[kylin] branch kylin5 updated: KYLIN-5274 Improve performance of getSubstitutor
This is an automated email from the ASF dual-hosted git repository.
xxyu pushed a commit to branch kylin5
in repository https://gitbox.apache.org/repos/asf/kylin.git
The following commit(s) were added to refs/heads/kylin5 by this push:
new 88b5a43813 KYLIN-5274 Improve performance of getSubstitutor
88b5a43813 is described below
commit 88b5a43813af205171c6bd30bce20f9d58fbff3a
Author: Zhong Yanghong <ya...@ebay.com>
AuthorDate: Sat Oct 8 17:56:17 2022 +0800
KYLIN-5274 Improve performance of getSubstitutor
---
.../kylin/common/ICachedExternalConfigLoader.java | 27 ++
.../org/apache/kylin/common/KylinConfigBase.java | 11 +-
.../org/apache/kylin/common/KylinConfigExt.java | 5 +-
.../kylin/common/KylinExternalConfigLoader.java | 20 +-
.../apache/kylin/common/PropertiesDelegate.java | 178 +++++++++--
.../apache/kylin/common/util/CompositeMapView.java | 348 +++++++++++++++++++++
6 files changed, 554 insertions(+), 35 deletions(-)
diff --git a/src/core-common/src/main/java/org/apache/kylin/common/ICachedExternalConfigLoader.java b/src/core-common/src/main/java/org/apache/kylin/common/ICachedExternalConfigLoader.java
new file mode 100644
index 0000000000..9af569f8d1
--- /dev/null
+++ b/src/core-common/src/main/java/org/apache/kylin/common/ICachedExternalConfigLoader.java
@@ -0,0 +1,27 @@
+/*
+ * 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 org.apache.kylin.common;
+
+import com.google.common.collect.ImmutableMap;
+
+import io.kyligence.config.core.loader.IExternalConfigLoader;
+
+public interface ICachedExternalConfigLoader extends IExternalConfigLoader {
+ ImmutableMap getPropertyEntries();
+}
diff --git a/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java b/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
index 57cd5e6306..fb279cc0f4 100644
--- a/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
+++ b/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
@@ -65,6 +65,7 @@ import org.apache.kylin.common.constant.NonCustomProjectLevelConfig;
import org.apache.kylin.common.persistence.metadata.HDFSMetadataStore;
import org.apache.kylin.common.util.AddressUtil;
import org.apache.kylin.common.util.ClusterConstant;
+import org.apache.kylin.common.util.CompositeMapView;
import org.apache.kylin.common.util.EncryptUtil;
import org.apache.kylin.common.util.FileUtils;
import org.apache.kylin.common.util.SizeConvertUtil;
@@ -215,6 +216,7 @@ public abstract class KylinConfigBase implements Serializable {
* only reload properties
*/
volatile PropertiesDelegate properties;
+ volatile StrSubstitutor substitutor;
protected KylinConfigBase(IExternalConfigLoader configLoader) {
this(new Properties(), configLoader);
@@ -231,6 +233,8 @@ public abstract class KylinConfigBase implements Serializable {
this.properties = force ? new PropertiesDelegate(props, configLoader)
: new PropertiesDelegate(BCC.check(props), configLoader);
}
+ // env > properties
+ this.substitutor = new StrSubstitutor(new CompositeMapView(this.properties, STATIC_SYSTEM_ENV));
}
protected final String getOptional(String prop) {
@@ -265,12 +269,7 @@ public abstract class KylinConfigBase implements Serializable {
}
protected StrSubstitutor getSubstitutor() {
- // env > properties
- final Map<String, Object> all = Maps.newHashMap();
- all.putAll((Map) properties);
- all.putAll(STATIC_SYSTEM_ENV);
-
- return new StrSubstitutor(all);
+ return substitutor;
}
protected Properties getRawAllProperties() {
diff --git a/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigExt.java b/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigExt.java
index 3f9a148656..7f2badb819 100644
--- a/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigExt.java
+++ b/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigExt.java
@@ -109,7 +109,10 @@ public class KylinConfigExt extends KylinConfig {
protected StrSubstitutor getSubstitutor() {
// overrides > env > properties
final Map<String, Object> all = Maps.newHashMap();
- all.putAll((Map) properties);
+ // all.putAll((Map) properties); // use putAll will call CompositeMapView.size()sss
+ for(Map.Entry<Object, Object> e: properties.entrySet()) {
+ all.put(e.getKey().toString(), e.getValue());
+ }
all.putAll(STATIC_SYSTEM_ENV);
all.putAll(overrides);
return new StrSubstitutor(all);
diff --git a/src/core-common/src/main/java/org/apache/kylin/common/KylinExternalConfigLoader.java b/src/core-common/src/main/java/org/apache/kylin/common/KylinExternalConfigLoader.java
index fb04d7bbdf..18dece3844 100644
--- a/src/core-common/src/main/java/org/apache/kylin/common/KylinExternalConfigLoader.java
+++ b/src/core-common/src/main/java/org/apache/kylin/common/KylinExternalConfigLoader.java
@@ -43,10 +43,9 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
-import io.kyligence.config.core.loader.IExternalConfigLoader;
-
-public class KylinExternalConfigLoader implements IExternalConfigLoader {
+public class KylinExternalConfigLoader implements ICachedExternalConfigLoader {
private static final long serialVersionUID = 1694879531312203159L;
@@ -58,6 +57,8 @@ public class KylinExternalConfigLoader implements IExternalConfigLoader {
private final Properties properties;
+ private final ImmutableMap<Object, Object> propertyEntries;
+
public KylinExternalConfigLoader(Map<String, String> map) {
this(map.get("config-dir") == null ? getSitePropertiesFile() : new File(map.get("config-dir")));
}
@@ -65,6 +66,7 @@ public class KylinExternalConfigLoader implements IExternalConfigLoader {
public KylinExternalConfigLoader(File file) {
this.propFile = file;
this.properties = loadProperties();
+ this.propertyEntries = ImmutableMap.copyOf(properties);
}
private Properties loadProperties() {
@@ -161,11 +163,21 @@ public class KylinExternalConfigLoader implements IExternalConfigLoader {
@Override
public String getProperty(String key) {
- return properties.getProperty(key);
+ Object oval = propertyEntries.get(key);
+ return (oval instanceof String) ? (String) oval : null;
}
+ /**
+ * @see #getPropertyEntries
+ */
@Override
+ @Deprecated
public Properties getProperties() {
return this.properties;
}
+
+ @Override
+ public ImmutableMap getPropertyEntries() {
+ return propertyEntries;
+ }
}
diff --git a/src/core-common/src/main/java/org/apache/kylin/common/PropertiesDelegate.java b/src/core-common/src/main/java/org/apache/kylin/common/PropertiesDelegate.java
index fd24fbd98b..bbb8add63b 100644
--- a/src/core-common/src/main/java/org/apache/kylin/common/PropertiesDelegate.java
+++ b/src/core-common/src/main/java/org/apache/kylin/common/PropertiesDelegate.java
@@ -18,18 +18,30 @@
package org.apache.kylin.common;
-import com.google.common.collect.Maps;
-import io.kyligence.config.core.loader.IExternalConfigLoader;
-import io.kyligence.config.external.loader.NacosExternalConfigLoader;
-import lombok.EqualsAndHashCode;
-
+import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
import java.util.Map;
import java.util.Properties;
import java.util.Set;
import java.util.concurrent.ConcurrentMap;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+import io.kyligence.config.core.loader.IExternalConfigLoader;
+import org.apache.kylin.common.util.CompositeMapView;
+
+import com.google.common.collect.Maps;
+
+import io.kyligence.config.external.loader.NacosExternalConfigLoader;
+import lombok.EqualsAndHashCode;
+/**
+ * It's mainly for reading by getting property config for some key.
+ * A few functions of hashtable are disabled.
+ * In the future, we should replace the java.util.Properties
+ */
@EqualsAndHashCode
public class PropertiesDelegate extends Properties {
@@ -39,34 +51,84 @@ public class PropertiesDelegate extends Properties {
@EqualsAndHashCode.Include
private final transient IExternalConfigLoader configLoader;
+ private final Map delegation;
+
public PropertiesDelegate(Properties properties, IExternalConfigLoader configLoader) {
- if(configLoader != null) this.properties.putAll(configLoader.getProperties());
this.properties.putAll(properties);
this.configLoader = configLoader;
+ if (configLoader == null) {
+ this.delegation = this.properties;
+ } else if (configLoader instanceof KylinExternalConfigLoader) {
+ this.delegation = new CompositeMapView(((ICachedExternalConfigLoader)this.configLoader).getPropertyEntries(), this.properties);
+ } else if (configLoader instanceof NacosExternalConfigLoader) {
+ this.delegation = new CompositeMapView((this.configLoader).getProperties(), this.properties);
+ } else {
+ throw new IllegalArgumentException(configLoader.getClass() + " is not supported ");
+ }
}
public void reloadProperties(Properties properties) {
this.properties.clear();
- if(configLoader != null) this.properties.putAll(configLoader.getProperties());
this.properties.putAll(properties);
}
@Override
public String getProperty(String key) {
- String property = (String) this.properties.get(key);
- if (property == null && this.configLoader != null) {
- return configLoader.getProperty(key);
- }
- return property;
+ return (String) this.get(key);
}
@Override
public String getProperty(String key, String defaultValue) {
- String property = this.getProperty(key);
- if (property == null) {
- return defaultValue;
- }
- return property;
+ return (String) this.getOrDefault(key, defaultValue);
+ }
+
+ @Override
+ public Object setProperty(String key, String value) {
+ return this.put(key, value);
+ }
+
+ @Override
+ public Enumeration<Object> keys() {
+ return Collections.enumeration(delegation.keySet());
+ }
+
+ @Override
+ public Enumeration<Object> elements() {
+ return Collections.enumeration(delegation.values());
+ }
+
+ @Override
+ public boolean contains(Object value) {
+ throw new UnsupportedOperationException();
+ }
+
+ /**
+ * It's not accurate since overridden keys will be counted multiple times
+ */
+ @Override
+ public int size() {
+ return delegation.size();
+ }
+
+
+ @Override
+ public boolean isEmpty() {
+ return delegation.isEmpty();
+ }
+
+ @Override
+ public boolean containsKey(Object key) {
+ return delegation.containsKey(key);
+ }
+
+ @Override
+ public boolean containsValue(Object value) {
+ return delegation.containsValue(value);
+ }
+
+ @Override
+ public Object get(Object key) {
+ return delegation.get(key);
}
@Override
@@ -75,23 +137,53 @@ public class PropertiesDelegate extends Properties {
}
@Override
- public Object setProperty(String key, String value) {
- return this.put(key, value);
+ public Object remove(Object key) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public void putAll(Map<?, ?> t) {
+ properties.putAll(t);
+ }
+
+ @Override
+ public void clear() {
+ properties.clear();
+ }
+
+ @Override
+ public Object clone() {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public String toString() {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Set<Object> keySet() {
+ return delegation.keySet();
}
@Override
public Set<Map.Entry<Object, Object>> entrySet() {
- return getAllProperties().entrySet();
+ return delegation.entrySet();
}
@Override
- public int size() {
- return getAllProperties().size();
+ public Collection<Object> values() {
+ return delegation.values();
}
@Override
- public Enumeration<Object> keys() {
- return Collections.enumeration(getAllProperties().keySet());
+ public Object getOrDefault(Object key, Object defaultValue) {
+ return delegation.getOrDefault(key, defaultValue);
+ }
+
+ @Override
+ public void forEach(BiConsumer<? super Object, ? super Object> action) {
+ throw new UnsupportedOperationException();
}
private ConcurrentMap<Object, Object> getAllProperties() {
@@ -117,4 +209,42 @@ public class PropertiesDelegate extends Properties {
throw new IllegalArgumentException(configLoader.getClass() + " is not supported ");
}
}
+
+ @Override
+ public boolean remove(Object key, Object value) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public boolean replace(Object key, Object oldValue, Object newValue) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Object replace(Object key, Object value) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Object computeIfAbsent(Object key, Function<? super Object, ? extends Object> mappingFunction) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Object computeIfPresent(Object key,
+ BiFunction<? super Object, ? super Object, ? extends Object> remappingFunction) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public synchronized Object compute(Object key,
+ BiFunction<? super Object, ? super Object, ? extends Object> remappingFunction) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public synchronized Object merge(Object key, Object value,
+ BiFunction<? super Object, ? super Object, ? extends Object> remappingFunction) {
+ throw new UnsupportedOperationException();
+ }
}
diff --git a/src/core-common/src/main/java/org/apache/kylin/common/util/CompositeMapView.java b/src/core-common/src/main/java/org/apache/kylin/common/util/CompositeMapView.java
new file mode 100644
index 0000000000..d0ed4d3df2
--- /dev/null
+++ b/src/core-common/src/main/java/org/apache/kylin/common/util/CompositeMapView.java
@@ -0,0 +1,348 @@
+/*
+ * 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 org.apache.kylin.common.util;
+
+import java.lang.reflect.Array;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+
+import org.apache.commons.collections.collection.CompositeCollection;
+import org.apache.commons.collections.iterators.EmptyIterator;
+import org.apache.commons.collections.iterators.IteratorChain;
+
+/**
+ * <pre>
+ * Decorates a map of some maps to provide a single unified view.
+ * 1. The updatable methods for CompositeMapView is not supported.
+ * 2. The changes of the composite maps can be reflected in this view
+ * </pre>
+ */
+public class CompositeMapView implements Map {
+
+ /**
+ * Array of all maps in the composite,
+ * latter element has higher priority to be read and write
+ * */
+ private final Map[] composite;
+
+ public CompositeMapView(Map one, Map two) {
+ this(new Map[] { one, two });
+ }
+
+ public CompositeMapView(Map[] composite) {
+ this.composite = composite;
+ }
+
+
+ @Override
+ public int size() {
+ throw new UnsupportedOperationException("Use size(boolean precise) instead.");
+ }
+
+ public int size(boolean precise) {
+ if (!precise) {
+ Set<Object> set = new HashSet<>();
+ for (int i = this.composite.length - 1; i >= 0; --i) {
+ set.addAll(this.composite[i].keySet());
+ }
+ return set.size();
+ } else {
+ int count = 0;
+ for (int i = this.composite.length - 1; i >= 0; --i) {
+ count += this.composite[i].size();
+ }
+ return count;
+ }
+ }
+
+ @Override
+ public boolean isEmpty() {
+ for (int i = this.composite.length - 1; i >= 0; --i) {
+ if (!this.composite[i].isEmpty()) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ @Override
+ public boolean containsKey(Object key) {
+ for (int i = this.composite.length - 1; i >= 0; --i) {
+ if (this.composite[i].containsKey(key)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public boolean containsValue(Object value) {
+ for (int i = this.composite.length - 1; i >= 0; --i) {
+ if (this.composite[i].containsValue(value)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public Object get(Object key) {
+ for (int i = this.composite.length - 1; i >= 0; --i) {
+ if (this.composite[i].containsKey(key)) {
+ return this.composite[i].get(key);
+ }
+ }
+ return null;
+ }
+
+ @Override
+ public Object put(Object key, Object value) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Object remove(Object key) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public void putAll(Map t) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public void clear() {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Object clone() {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public String toString() {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Set<Object> keySet() {
+ Set[] keys = new Set[composite.length];
+ for (int i = 0; i < composite.length; i++) {
+ keys[i] = this.composite[i].keySet();
+ }
+ return new CompositeSetView(keys);
+ }
+
+ @Override
+ public Set<Map.Entry<Object, Object>> entrySet() {
+ Set[] entries = new Set[composite.length];
+ for (int i = 0; i < composite.length; i++) {
+ entries[i] = this.composite[i].entrySet();
+ }
+ return new CompositeSetView(entries);
+ }
+
+ @Override
+ public Collection<Object> values() {
+ Collection[] values = new Collection[composite.length];
+ for (int i = 0; i < composite.length; i++) {
+ values[i] = this.composite[i].values();
+ }
+ return new CompositeCollectionView(values);
+ }
+
+ @Override
+ public void forEach(BiConsumer action) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public void replaceAll(BiFunction function) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Object putIfAbsent(Object key, Object value) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public boolean remove(Object key, Object value) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public boolean replace(Object key, Object oldValue, Object newValue) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Object replace(Object key, Object value) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Object computeIfAbsent(Object key, Function mappingFunction) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Object computeIfPresent(Object key, BiFunction remappingFunction) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public synchronized Object compute(Object key, BiFunction remappingFunction) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public synchronized Object merge(Object key, Object value, BiFunction remappingFunction) {
+ throw new UnsupportedOperationException();
+ }
+
+ private static class CompositeCollectionView implements Collection {
+ protected Collection[] all;
+
+ public CompositeCollectionView(Collection[] colls) {
+ this.all = colls;
+ }
+
+ @Override
+ public int size() {
+ int size = 0;
+ for (int i = this.all.length - 1; i >= 0; i--) {
+ size += this.all[i].size();
+ }
+ return size;
+ }
+
+ @Override
+ public boolean isEmpty() {
+ for (int i = this.all.length - 1; i >= 0; i--) {
+ if (!this.all[i].isEmpty()) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ @Override
+ public boolean contains(Object obj) {
+ for (int i = this.all.length - 1; i >= 0; i--) {
+ if (this.all[i].contains(obj)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public Iterator iterator() {
+ if (this.all.length == 0) {
+ return EmptyIterator.INSTANCE;
+ }
+ IteratorChain chain = new IteratorChain();
+ for (int i = 0; i < this.all.length; ++i) {
+ chain.addIterator(this.all[i].iterator());
+ }
+ return chain;
+ }
+
+ @Override
+ public Object[] toArray() {
+ final Object[] result = new Object[this.size()];
+ int i = 0;
+ for (Iterator it = this.iterator(); it.hasNext(); i++) {
+ result[i] = it.next();
+ }
+ return result;
+ }
+
+ @Override
+ public boolean add(Object o) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public boolean remove(Object o) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public boolean addAll(Collection c) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public void clear() {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public boolean retainAll(Collection c) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public boolean removeAll(Collection c) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public boolean containsAll(Collection c) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Object[] toArray(Object[] array) {
+ int size = this.size();
+ Object[] result = null;
+ if (array.length >= size) {
+ result = array;
+ } else {
+ result = (Object[]) Array.newInstance(array.getClass().getComponentType(), size);
+ }
+
+ int offset = 0;
+ for (int i = 0; i < this.all.length; ++i) {
+ for (Iterator it = this.all[i].iterator(); it.hasNext();) {
+ result[offset++] = it.next();
+ }
+ }
+ if (result.length > size) {
+ result[size] = null;
+ }
+ return result;
+ }
+ }
+
+ private static class CompositeSetView extends CompositeCollection implements Set {
+ public CompositeSetView(Collection[] colls) {
+ super(colls);
+ }
+ }
+}