Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ISSUE #11294] Optimize ConcurrentHashMap#computeIfAbsent have performance problem in jdk1.8 #11326

Merged
merged 4 commits into from
Jan 19, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.apache.dubbo.rpc.cluster;

import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.utils.ConcurrentHashMapUtils;

import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
Expand All @@ -30,7 +31,7 @@ public abstract class CacheableRouterFactory implements RouterFactory {

@Override
public Router getRouter(URL url) {
return routerMap.computeIfAbsent(url.getServiceKey(), k -> createRouter(url));
return ConcurrentHashMapUtils.computeIfAbsent(routerMap, url.getServiceKey(), k -> createRouter(url));
}

protected abstract Router createRouter(URL url);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.apache.dubbo.rpc.cluster.loadbalance;

import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.utils.ConcurrentHashMapUtils;
import org.apache.dubbo.rpc.Invocation;
import org.apache.dubbo.rpc.Invoker;

Expand All @@ -35,6 +36,8 @@ public class RoundRobinLoadBalance extends AbstractLoadBalance {

private static final int RECYCLE_PERIOD = 60000;

private ConcurrentMap<String, ConcurrentMap<String, WeightedRoundRobin>> methodWeightMap = new ConcurrentHashMap<>();

protected static class WeightedRoundRobin {
private int weight;
private AtomicLong current = new AtomicLong(0);
Expand Down Expand Up @@ -66,8 +69,6 @@ public void setLastUpdate(long lastUpdate) {
}
}

private ConcurrentMap<String, ConcurrentMap<String, WeightedRoundRobin>> methodWeightMap = new ConcurrentHashMap<String, ConcurrentMap<String, WeightedRoundRobin>>();

/**
* get invoker addr list cached for specified invocation
* <p>
Expand All @@ -89,7 +90,7 @@ protected <T> Collection<String> getInvokerAddrList(List<Invoker<T>> invokers, I
@Override
protected <T> Invoker<T> doSelect(List<Invoker<T>> invokers, URL url, Invocation invocation) {
String key = invokers.get(0).getUrl().getServiceKey() + "." + invocation.getMethodName();
ConcurrentMap<String, WeightedRoundRobin> map = methodWeightMap.computeIfAbsent(key, k -> new ConcurrentHashMap<>());
ConcurrentMap<String, WeightedRoundRobin> map = ConcurrentHashMapUtils.computeIfAbsent(methodWeightMap, key, k -> new ConcurrentHashMap<>());
int totalWeight = 0;
long maxCurrent = Long.MIN_VALUE;
long now = System.currentTimeMillis();
Expand All @@ -98,7 +99,7 @@ protected <T> Invoker<T> doSelect(List<Invoker<T>> invokers, URL url, Invocation
for (Invoker<T> invoker : invokers) {
String identifyString = invoker.getUrl().toIdentityString();
int weight = getWeight(invoker, invocation);
WeightedRoundRobin weightedRoundRobin = map.computeIfAbsent(identifyString, k -> {
WeightedRoundRobin weightedRoundRobin = ConcurrentHashMapUtils.computeIfAbsent(map, identifyString, k -> {
WeightedRoundRobin wrr = new WeightedRoundRobin();
wrr.setWeight(weight);
return wrr;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.threadpool.manager.FrameworkExecutorRepository;
import org.apache.dubbo.common.utils.ConcurrentHashMapUtils;
import org.apache.dubbo.rpc.Invocation;
import org.apache.dubbo.rpc.Invoker;
import org.apache.dubbo.rpc.RpcStatus;
Expand Down Expand Up @@ -116,7 +117,7 @@ protected <T> Invoker<T> doSelect(List<Invoker<T>> invokers, URL url, Invocation
for (int i = 0; i < length; i++) {
Invoker<T> invoker = invokers.get(i);
RpcStatus rpcStatus = RpcStatus.getStatus(invoker.getUrl(), invocation.getMethodName());
SlideWindowData slideWindowData = methodMap.computeIfAbsent(rpcStatus, SlideWindowData::new);
SlideWindowData slideWindowData = ConcurrentHashMapUtils.computeIfAbsent(methodMap, rpcStatus, SlideWindowData::new);

// Calculate the estimated response time from the product of active connections and succeeded average elapsed time.
long estimateResponse = slideWindowData.getEstimateResponse();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@
import org.apache.dubbo.common.logger.ErrorTypeAwareLogger;
import org.apache.dubbo.common.logger.LoggerFactory;
import org.apache.dubbo.common.utils.CollectionUtils;
import org.apache.dubbo.common.utils.ConcurrentHashMapUtils;
import org.apache.dubbo.common.utils.ConcurrentHashSet;

import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;

import static org.apache.dubbo.common.constants.LoggerCodeConstants.CLUSTER_NO_RULE_LISTENER;

Expand All @@ -34,7 +36,7 @@ public class MeshRuleDispatcher {
private static final ErrorTypeAwareLogger logger = LoggerFactory.getErrorTypeAwareLogger(MeshRuleDispatcher.class);

private final String appName;
private final Map<String, Set<MeshRuleListener>> listenerMap = new ConcurrentHashMap<>();
private final ConcurrentMap<String, Set<MeshRuleListener>> listenerMap = new ConcurrentHashMap<>();

public MeshRuleDispatcher(String appName) {
this.appName = appName;
Expand All @@ -57,7 +59,7 @@ public synchronized void post(Map<String, List<Map<String, Object>>> ruleMap) {
listener.onRuleChange(appName, entry.getValue());
}
} else {
logger.warn(CLUSTER_NO_RULE_LISTENER,"Receive mesh rule but none of listener has been registered","","Receive rule but none of listener has been registered. Maybe type not matched. Rule Type: " + ruleType);
logger.warn(CLUSTER_NO_RULE_LISTENER, "Receive mesh rule but none of listener has been registered", "", "Receive rule but none of listener has been registered. Maybe type not matched. Rule Type: " + ruleType);
}
}
// clear rule listener not being notified in this time
Expand All @@ -75,7 +77,7 @@ public synchronized void register(MeshRuleListener listener) {
if (listener == null) {
return;
}
listenerMap.computeIfAbsent(listener.ruleSuffix(), (k) -> new ConcurrentHashSet<>())
ConcurrentHashMapUtils.computeIfAbsent(listenerMap, listener.ruleSuffix(), (k) -> new ConcurrentHashSet<>())
.add(listener);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.logger.ErrorTypeAwareLogger;
import org.apache.dubbo.common.logger.LoggerFactory;
import org.apache.dubbo.common.utils.ConcurrentHashMapUtils;
import org.apache.dubbo.common.utils.Holder;
import org.apache.dubbo.common.utils.StringUtils;
import org.apache.dubbo.rpc.Invocation;
Expand Down Expand Up @@ -47,6 +48,7 @@
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.stream.Collectors;

import static org.apache.dubbo.common.constants.LoggerCodeConstants.CLUSTER_SCRIPT_EXCEPTION;
Expand All @@ -64,7 +66,7 @@ public class ScriptStateRouter<T> extends AbstractStateRouter<T> {
private static final int SCRIPT_ROUTER_DEFAULT_PRIORITY = 0;
private static final ErrorTypeAwareLogger logger = LoggerFactory.getErrorTypeAwareLogger(ScriptStateRouter.class);

private static final Map<String, ScriptEngine> ENGINES = new ConcurrentHashMap<>();
private static final ConcurrentMap<String, ScriptEngine> ENGINES = new ConcurrentHashMap<>();

private final ScriptEngine engine;

Expand Down Expand Up @@ -93,8 +95,8 @@ public ScriptStateRouter(URL url) {
Compilable compilable = (Compilable) engine;
function = compilable.compile(rule);
} catch (ScriptException e) {
logger.error(CLUSTER_SCRIPT_EXCEPTION,"script route rule invalid","","script route error, rule has been ignored. rule: " + rule +
", url: " + RpcContext.getServiceContext().getUrl(),e);
logger.error(CLUSTER_SCRIPT_EXCEPTION, "script route rule invalid", "", "script route error, rule has been ignored. rule: " + rule +
", url: " + RpcContext.getServiceContext().getUrl(), e);
}
}

Expand All @@ -115,7 +117,7 @@ private String getRule(URL url) {
private ScriptEngine getEngine(URL url) {
String type = url.getParameter(TYPE_KEY, DEFAULT_SCRIPT_TYPE_KEY);

return ENGINES.computeIfAbsent(type, t -> {
return ConcurrentHashMapUtils.computeIfAbsent(ENGINES, type, t -> {
ScriptEngine scriptEngine = new ScriptEngineManager().getEngineByName(type);
if (scriptEngine == null) {
throw new IllegalStateException("unsupported route engine type: " + type);
Expand All @@ -137,8 +139,8 @@ protected BitList<Invoker<T>> doRoute(BitList<Invoker<T>> invokers, URL url, Inv
try {
return function.eval(bindings);
} catch (ScriptException e) {
logger.error(CLUSTER_SCRIPT_EXCEPTION,"Scriptrouter exec script error","","Script route error, rule has been ignored. rule: " + rule + ", method:" +
invocation.getMethodName() + ", url: " + RpcContext.getContext().getUrl(),e);
logger.error(CLUSTER_SCRIPT_EXCEPTION, "Scriptrouter exec script error", "", "Script route error, rule has been ignored. rule: " + rule + ", method:" +
invocation.getMethodName() + ", url: " + RpcContext.getContext().getUrl(), e);
return invokers;
}
}, accessControlContext));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.apache.dubbo.rpc.cluster.router.state;

import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.utils.ConcurrentHashMapUtils;

import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
Expand All @@ -31,7 +32,7 @@ public abstract class CacheableStateRouterFactory implements StateRouterFactory

@Override
public <T> StateRouter<T> getRouter(Class<T> interfaceClass, URL url) {
return routerMap.computeIfAbsent(url.getServiceKey(), k -> createRouter(interfaceClass, url));
return ConcurrentHashMapUtils.computeIfAbsent(routerMap, url.getServiceKey(), k -> createRouter(interfaceClass, url));
}

protected abstract <T> StateRouter<T> createRouter(Class<T> interfaceClass, URL url);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.apache.dubbo.common.logger.ErrorTypeAwareLogger;
import org.apache.dubbo.common.logger.LoggerFactory;
import org.apache.dubbo.common.resource.Disposable;
import org.apache.dubbo.common.utils.ConcurrentHashMapUtils;
import org.apache.dubbo.common.utils.StringUtils;
import org.apache.dubbo.rpc.model.ScopeModelAccessor;

Expand All @@ -49,7 +50,7 @@ public class ScopeBeanFactory {
private final ScopeBeanFactory parent;
private ExtensionAccessor extensionAccessor;
private List<ExtensionPostProcessor> extensionPostProcessors;
private Map<Class, AtomicInteger> beanNameIdCounterMap = new ConcurrentHashMap<>();
private ConcurrentHashMap<Class, AtomicInteger> beanNameIdCounterMap = new ConcurrentHashMap<>();
private List<BeanInfo> registeredBeanInfos = new CopyOnWriteArrayList<>();
private InstantiationStrategy instantiationStrategy;
private AtomicBoolean destroyed = new AtomicBoolean();
Expand Down Expand Up @@ -183,7 +184,7 @@ private boolean containsBean(String name, Object bean) {
}

private int getNextId(Class<?> beanClass) {
return beanNameIdCounterMap.computeIfAbsent(beanClass, key -> new AtomicInteger()).incrementAndGet();
return ConcurrentHashMapUtils.computeIfAbsent(beanNameIdCounterMap, beanClass, key -> new AtomicInteger()).incrementAndGet();
}

public <T> T getBean(Class<T> type) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.apache.dubbo.common.bytecode;

import org.apache.dubbo.common.utils.ClassUtils;
import org.apache.dubbo.common.utils.ConcurrentHashMapUtils;
import org.apache.dubbo.common.utils.ReflectUtils;

import javassist.ClassPool;
Expand All @@ -34,6 +35,7 @@
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.atomic.AtomicLong;
import java.util.regex.Matcher;
import java.util.stream.Collectors;
Expand All @@ -42,7 +44,7 @@
* Wrapper.
*/
public abstract class Wrapper {
private static final Map<Class<?>, Wrapper> WRAPPER_MAP = new ConcurrentHashMap<Class<?>, Wrapper>(); //class wrapper map
private static final ConcurrentMap<Class<?>, Wrapper> WRAPPER_MAP = new ConcurrentHashMap<Class<?>, Wrapper>(); //class wrapper map
private static final String[] EMPTY_STRING_ARRAY = new String[0];
private static final String[] OBJECT_METHODS = new String[]{"getClass", "hashCode", "toString", "equals"};
private static final Wrapper OBJECT_WRAPPER = new Wrapper() {
Expand Down Expand Up @@ -119,7 +121,7 @@ public static Wrapper getWrapper(Class<?> c) {
return OBJECT_WRAPPER;
}

return WRAPPER_MAP.computeIfAbsent(c, Wrapper::makeWrapper);
return ConcurrentHashMapUtils.computeIfAbsent(WRAPPER_MAP, c, Wrapper::makeWrapper);
}

private static Wrapper makeWrapper(Class<?> c) {
Expand Down Expand Up @@ -171,16 +173,16 @@ private static Wrapper makeWrapper(Class<?> c) {
}

Method[] methods = Arrays.stream(c.getMethods())
.filter(method -> allMethod.contains(ReflectUtils.getDesc(method)))
.collect(Collectors.toList())
.toArray(new Method[] {});
.filter(method -> allMethod.contains(ReflectUtils.getDesc(method)))
.collect(Collectors.toList())
.toArray(new Method[]{});
// get all public method.
boolean hasMethod = ClassUtils.hasMethods(methods);
if (hasMethod) {
Map<String, Integer> sameNameMethodCount = new HashMap<>((int) (methods.length / 0.75f) + 1);
for (Method m : methods) {
sameNameMethodCount.compute(m.getName(),
(key, oldValue) -> oldValue == null ? 1 : oldValue + 1);
(key, oldValue) -> oldValue == null ? 1 : oldValue + 1);
}

c3.append(" try{");
Expand All @@ -200,7 +202,7 @@ private static Wrapper makeWrapper(Class<?> c) {
if (len > 0) {
for (int l = 0; l < len; l++) {
c3.append(" && ").append(" $3[").append(l).append("].getName().equals(\"")
.append(m.getParameterTypes()[l].getName()).append("\")");
.append(m.getParameterTypes()[l].getName()).append("\")");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import org.apache.dubbo.common.logger.ErrorTypeAwareLogger;
import org.apache.dubbo.common.logger.LoggerFactory;
import org.apache.dubbo.common.utils.ConcurrentHashMapUtils;

import java.io.File;
import java.io.IOException;
Expand All @@ -33,6 +34,7 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;

import static org.apache.dubbo.common.constants.LoggerCodeConstants.COMMON_CACHE_PATH_INACCESSIBLE;

Expand All @@ -50,11 +52,11 @@ private FileCacheStoreFactory() {
}

private static final ErrorTypeAwareLogger logger = LoggerFactory.getErrorTypeAwareLogger(FileCacheStoreFactory.class);
private static final Map<String, FileCacheStore> cacheMap = new ConcurrentHashMap<>();
private static final ConcurrentMap<String, FileCacheStore> cacheMap = new ConcurrentHashMap<>();

private static final String SUFFIX = ".dubbo.cache";
private static final char ESCAPE_MARK = '%';
private static final Set<Character> LEGAL_CHARACTERS = Collections.unmodifiableSet(new HashSet<Character>(){{
private static final Set<Character> LEGAL_CHARACTERS = Collections.unmodifiableSet(new HashSet<Character>() {{
// - $ . _ 0-9 a-z A-Z
add('-');
add('$');
Expand Down Expand Up @@ -108,7 +110,7 @@ public static FileCacheStore getInstance(String basePath, String cacheName, bool

String cacheFilePath = basePath + File.separator + cacheName;

return cacheMap.computeIfAbsent(cacheFilePath, k -> getFile(k, enableFileCache));
return ConcurrentHashMapUtils.computeIfAbsent(cacheMap, cacheFilePath, k -> getFile(k, enableFileCache));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.apache.dubbo.common.config.configcenter;

import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.utils.ConcurrentHashMapUtils;

import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
Expand All @@ -31,12 +32,12 @@
*/
public abstract class AbstractDynamicConfigurationFactory implements DynamicConfigurationFactory {

private volatile Map<String, DynamicConfiguration> dynamicConfigurations = new ConcurrentHashMap<>();
private volatile ConcurrentHashMap<String, DynamicConfiguration> dynamicConfigurations = new ConcurrentHashMap<>();

@Override
public final DynamicConfiguration getDynamicConfiguration(URL url) {
String key = url == null ? DEFAULT_KEY : url.toServiceString();
return dynamicConfigurations.computeIfAbsent(key, k -> createDynamicConfiguration(url));
return ConcurrentHashMapUtils.computeIfAbsent(dynamicConfigurations, key, k -> createDynamicConfiguration(url));
}

protected abstract DynamicConfiguration createDynamicConfiguration(URL url);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,18 @@
package org.apache.dubbo.common.convert;

import org.apache.dubbo.common.extension.ExtensionLoader;
import org.apache.dubbo.common.utils.ConcurrentHashMapUtils;
import org.apache.dubbo.rpc.model.FrameworkModel;

import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.stream.Collectors;

public class ConverterUtil {
private final FrameworkModel frameworkModel;
private final Map<Class<?>, Map<Class<?>, List<Converter>>> converterCache = new ConcurrentHashMap<>();
private final ConcurrentMap<Class<?>, ConcurrentMap<Class<?>, List<Converter>>> converterCache = new ConcurrentHashMap<>();

public ConverterUtil(FrameworkModel frameworkModel) {
this.frameworkModel = frameworkModel;
Expand All @@ -41,8 +43,8 @@ public ConverterUtil(FrameworkModel frameworkModel) {
* @see ExtensionLoader#getSupportedExtensionInstances()
*/
public Converter<?, ?> getConverter(Class<?> sourceType, Class<?> targetType) {
Map<Class<?>, List<Converter>> toTargetMap = converterCache.computeIfAbsent(sourceType, (k) -> new ConcurrentHashMap<>());
List<Converter> converters = toTargetMap.computeIfAbsent(targetType, (k) -> frameworkModel.getExtensionLoader(Converter.class)
ConcurrentMap<Class<?>, List<Converter>> toTargetMap = ConcurrentHashMapUtils.computeIfAbsent(converterCache, sourceType, (k) -> new ConcurrentHashMap<>());
List<Converter> converters = ConcurrentHashMapUtils.computeIfAbsent(toTargetMap, targetType, (k) -> frameworkModel.getExtensionLoader(Converter.class)
.getSupportedExtensionInstances()
.stream()
.filter(converter -> converter.accept(sourceType, targetType))
Expand Down
Loading