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 <yangzh...@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); + } + } +}