You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2020/05/26 09:43:35 UTC
[servicecomb-java-chassis] 02/02: [SCB-1930]using set for
priorityproperty cache and remove unnecessary code
This is an automated email from the ASF dual-hosted git repository.
liubao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git
commit 71fd15c87858e88532f26642d739caef77ad6d64
Author: liubao <bi...@qq.com>
AuthorDate: Tue May 26 10:40:52 2020 +0800
[SCB-1930]using set for priorityproperty cache and remove unnecessary code
---
.../core/definition/MicroserviceVersionsMeta.java | 5 -
.../core/definition/ServiceRegistryListener.java | 5 +-
.../config/priority/PriorityPropertyManager.java | 29 ++----
.../config/inject/TestConfigObjectFactory.java | 8 --
.../config/priority/TestPriorityProperty.java | 14 ---
.../config/priority/TestPriorityPropertyBase.java | 10 --
.../priority/TestPriorityPropertyManager.java | 109 +++++++++++++++++++++
.../inspector/internal/InspectorBootListener.java | 13 +--
.../inspector/internal/InspectorImpl.java | 2 +-
.../internal/TestInspectorBootListener.java | 1 -
.../inspector/internal/TestInspectorImpl.java | 6 --
11 files changed, 120 insertions(+), 82 deletions(-)
diff --git a/core/src/main/java/org/apache/servicecomb/core/definition/MicroserviceVersionsMeta.java b/core/src/main/java/org/apache/servicecomb/core/definition/MicroserviceVersionsMeta.java
index 307b51a..8252d2f 100644
--- a/core/src/main/java/org/apache/servicecomb/core/definition/MicroserviceVersionsMeta.java
+++ b/core/src/main/java/org/apache/servicecomb/core/definition/MicroserviceVersionsMeta.java
@@ -35,11 +35,6 @@ public class MicroserviceVersionsMeta {
.createConfigObject(MicroserviceConfig.class, "service", microserviceName);
}
- public void destroy() {
- scbEngine.getPriorityPropertyManager().unregisterConfigObject(microserviceConfig);
- configs.values().stream()
- .forEach(scbEngine.getPriorityPropertyManager()::unregisterConfigObject);
- }
public MicroserviceConfig getMicroserviceConfig() {
return microserviceConfig;
diff --git a/core/src/main/java/org/apache/servicecomb/core/definition/ServiceRegistryListener.java b/core/src/main/java/org/apache/servicecomb/core/definition/ServiceRegistryListener.java
index 0931864..5077234 100644
--- a/core/src/main/java/org/apache/servicecomb/core/definition/ServiceRegistryListener.java
+++ b/core/src/main/java/org/apache/servicecomb/core/definition/ServiceRegistryListener.java
@@ -64,10 +64,7 @@ public class ServiceRegistryListener {
@SubscriberOrder(-1000)
@Subscribe
public void onDestroyMicroservice(DestroyMicroserviceEvent event) {
- MicroserviceVersions microserviceVersions = event.getMicroserviceVersions();
- MicroserviceVersionsMeta microserviceVersionsMeta = microserviceVersions.getVendorExtensions()
- .get(CORE_MICROSERVICE_VERSIONS_META);
- microserviceVersionsMeta.destroy();
+
}
@EnableExceptionPropagation
diff --git a/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/priority/PriorityPropertyManager.java b/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/priority/PriorityPropertyManager.java
index a7d8544..dd84f5a 100644
--- a/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/priority/PriorityPropertyManager.java
+++ b/foundations/foundation-config/src/main/java/org/apache/servicecomb/config/priority/PriorityPropertyManager.java
@@ -23,6 +23,7 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.WeakHashMap;
import org.apache.commons.configuration.event.ConfigurationEvent;
@@ -30,15 +31,14 @@ import org.apache.commons.configuration.event.ConfigurationListener;
import org.apache.servicecomb.config.inject.ConfigObjectFactory;
import org.apache.servicecomb.foundation.common.concurrent.ConcurrentHashMapEx;
-import com.google.common.annotations.VisibleForTesting;
import com.netflix.config.ConfigurationManager;
import com.netflix.config.DynamicPropertyFactory;
public class PriorityPropertyManager {
private ConfigurationListener configurationListener = this::configurationListener;
- private Map<PriorityProperty<?>, Boolean> priorityPropertyMap =
- Collections.synchronizedMap(new WeakHashMap<>());
+ private Set<PriorityProperty<?>> priorityPropertySet =
+ Collections.newSetFromMap(Collections.synchronizedMap(new WeakHashMap<>()));
private Map<Object, List<PriorityProperty<?>>> configObjectMap =
Collections.synchronizedMap(new WeakHashMap<>());
@@ -66,7 +66,7 @@ public class PriorityPropertyManager {
if (keyCache == null) {
keyCache = new ConcurrentHashMapEx<>();
- updateCache(new Object(), priorityPropertyMap.keySet());
+ updateCache(new Object(), priorityPropertySet);
configObjectMap.forEach((k, v) -> updateCache(k, v));
}
@@ -92,8 +92,8 @@ public class PriorityPropertyManager {
}
}
- public Map<PriorityProperty<?>, Boolean> getPriorityPropertyMap() {
- return priorityPropertyMap;
+ public Set<PriorityProperty<?>> getPriorityPropertySet() {
+ return priorityPropertySet;
}
public Map<Object, List<PriorityProperty<?>>> getConfigObjectMap() {
@@ -101,7 +101,7 @@ public class PriorityPropertyManager {
}
private synchronized void registerPriorityProperty(PriorityProperty<?> property) {
- priorityPropertyMap.put(property, Boolean.TRUE);
+ priorityPropertySet.add(property);
keyCache = null;
}
@@ -110,20 +110,6 @@ public class PriorityPropertyManager {
keyCache = null;
}
- public synchronized void unregisterPriorityProperty(PriorityProperty<?> property) {
- priorityPropertyMap.remove(property);
- keyCache = null;
- }
-
- public synchronized void unregisterConfigObject(Object configObject) {
- if (configObject == null) {
- return;
- }
-
- configObjectMap.remove(configObject);
- keyCache = null;
- }
-
public <T> T createConfigObject(Class<T> cls, Object... kvs) {
Map<String, Object> parameters = new HashMap<>();
for (int idx = 0; idx < kvs.length; idx += 2) {
@@ -145,7 +131,6 @@ public class PriorityPropertyManager {
return new PriorityProperty<>(cls, invalidValue, defaultValue, priorityKeys);
}
- @VisibleForTesting
public <T> PriorityProperty<T> createPriorityProperty(Type cls, T invalidValue, T defaultValue,
String... priorityKeys) {
PriorityProperty<T> priorityProperty = new PriorityProperty<>(cls, invalidValue, defaultValue, priorityKeys);
diff --git a/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/inject/TestConfigObjectFactory.java b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/inject/TestConfigObjectFactory.java
index 7598e97..92cd17f 100644
--- a/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/inject/TestConfigObjectFactory.java
+++ b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/inject/TestConfigObjectFactory.java
@@ -190,7 +190,6 @@ public class TestConfigObjectFactory extends TestPriorityPropertyBase {
Assert.assertNull(config.booleanValueObj);
Assert.assertNull(config.getBooleanValueObj1());
- priorityPropertyManager.unregisterConfigObject(config);
}
@Test
@@ -253,7 +252,6 @@ public class TestConfigObjectFactory extends TestPriorityPropertyBase {
Assert.assertTrue(config.booleanValueObj);
Assert.assertTrue(config.getBooleanValueObj1());
- priorityPropertyManager.unregisterConfigObject(config);
}
@Test
@@ -316,7 +314,6 @@ public class TestConfigObjectFactory extends TestPriorityPropertyBase {
Assert.assertTrue(config.booleanValueObj);
Assert.assertTrue(config.getBooleanValueObj1());
- priorityPropertyManager.unregisterConfigObject(config);
}
@InjectProperties(prefix = "root")
@@ -363,7 +360,6 @@ public class TestConfigObjectFactory extends TestPriorityPropertyBase {
Assert.assertEquals(4, config.doubleDef, 0);
Assert.assertTrue(config.booleanDef);
- priorityPropertyManager.unregisterConfigObject(config);
}
@Test
@@ -390,7 +386,6 @@ public class TestConfigObjectFactory extends TestPriorityPropertyBase {
ArchaiusUtils.setProperty("root.low-1.a.high-1.b", Long.MAX_VALUE - 3);
Assert.assertEquals(Long.MAX_VALUE - 3, config.longValue);
- priorityPropertyManager.unregisterConfigObject(config);
}
@Test
@@ -406,7 +401,6 @@ public class TestConfigObjectFactory extends TestPriorityPropertyBase {
ArchaiusUtils.setProperty("root.l1-1", String.valueOf(2f));
Assert.assertEquals(2f, config.floatValue, 0);
- priorityPropertyManager.unregisterConfigObject(config);
}
@Test
@@ -418,7 +412,6 @@ public class TestConfigObjectFactory extends TestPriorityPropertyBase {
ArchaiusUtils.setProperty("root.k.value", "1");
Assert.assertEquals(1, config.intValue);
- priorityPropertyManager.unregisterConfigObject(config);
}
@Test
@@ -437,6 +430,5 @@ public class TestConfigObjectFactory extends TestPriorityPropertyBase {
ArchaiusUtils.setProperty("override.low", null);
Assert.assertNull(config.strValue);
- priorityPropertyManager.unregisterConfigObject(config);
}
}
diff --git a/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityProperty.java b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityProperty.java
index fe8604f..43356ed 100644
--- a/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityProperty.java
+++ b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityProperty.java
@@ -61,8 +61,6 @@ public class TestPriorityProperty extends TestPriorityPropertyBase {
ArchaiusUtils.setProperty(low, null);
Assert.assertEquals(-2L, (long) config.getValue());
-
- priorityPropertyManager.unregisterPriorityProperty(config);
}
@Test
@@ -91,8 +89,6 @@ public class TestPriorityProperty extends TestPriorityPropertyBase {
ArchaiusUtils.setProperty(low, null);
Assert.assertEquals(-2, (int) config.getValue());
-
- priorityPropertyManager.unregisterPriorityProperty(config);
}
@Test
@@ -121,8 +117,6 @@ public class TestPriorityProperty extends TestPriorityPropertyBase {
ArchaiusUtils.setProperty(low, null);
Assert.assertEquals("def", config.getValue());
-
- priorityPropertyManager.unregisterPriorityProperty(config);
}
@Test
@@ -151,8 +145,6 @@ public class TestPriorityProperty extends TestPriorityPropertyBase {
ArchaiusUtils.setProperty(low, null);
Assert.assertFalse(config.getValue());
-
- priorityPropertyManager.unregisterPriorityProperty(config);
}
@Test
@@ -181,8 +173,6 @@ public class TestPriorityProperty extends TestPriorityPropertyBase {
ArchaiusUtils.setProperty(low, null);
Assert.assertEquals(-2, config.getValue(), 0);
-
- priorityPropertyManager.unregisterPriorityProperty(config);
}
@Test
@@ -211,8 +201,6 @@ public class TestPriorityProperty extends TestPriorityPropertyBase {
ArchaiusUtils.setProperty(low, null);
Assert.assertEquals(-2, config.getValue(), 0);
-
- priorityPropertyManager.unregisterPriorityProperty(config);
}
@Test
@@ -226,7 +214,5 @@ public class TestPriorityProperty extends TestPriorityPropertyBase {
config.addConfiguration(new MapConfiguration(Collections.singletonMap(high, "high-value")));
Assert.assertEquals("high-value", property.getValue());
-
- priorityPropertyManager.unregisterPriorityProperty(property);
}
}
diff --git a/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityPropertyBase.java b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityPropertyBase.java
index 9603e15..841c8a9 100644
--- a/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityPropertyBase.java
+++ b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityPropertyBase.java
@@ -18,14 +18,10 @@ package org.apache.servicecomb.config.priority;
import org.apache.log4j.Level;
import org.apache.log4j.Logger;
-import org.apache.servicecomb.config.ConfigUtil;
import org.apache.servicecomb.foundation.test.scaffolding.config.ArchaiusUtils;
import org.junit.After;
-import org.junit.Assert;
import org.junit.Before;
-import com.netflix.config.DynamicProperty;
-
public class TestPriorityPropertyBase {
protected PriorityPropertyManager priorityPropertyManager;
@@ -42,12 +38,6 @@ public class TestPriorityPropertyBase {
@After
public void teardown() {
- Assert.assertTrue(priorityPropertyManager.getPriorityPropertyMap().isEmpty());
- Assert.assertTrue(priorityPropertyManager.getConfigObjectMap().isEmpty());
- for (DynamicProperty property : ConfigUtil.getAllDynamicProperties().values()) {
- Assert.assertTrue(ConfigUtil.getCallbacks(property).isEmpty());
- }
-
ArchaiusUtils.resetConfig();
}
}
diff --git a/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityPropertyManager.java b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityPropertyManager.java
new file mode 100644
index 0000000..bccf97f
--- /dev/null
+++ b/foundations/foundation-config/src/test/java/org/apache/servicecomb/config/priority/TestPriorityPropertyManager.java
@@ -0,0 +1,109 @@
+/*
+ * 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.servicecomb.config.priority;
+
+import org.apache.log4j.Level;
+import org.apache.log4j.Logger;
+import org.apache.servicecomb.config.ConfigUtil;
+import org.apache.servicecomb.config.inject.InjectProperties;
+import org.apache.servicecomb.config.inject.InjectProperty;
+import org.apache.servicecomb.config.inject.TestConfigObjectFactory;
+import org.apache.servicecomb.foundation.test.scaffolding.config.ArchaiusUtils;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.netflix.config.DynamicProperty;
+
+public class TestPriorityPropertyManager {
+ String high = "ms.schema.op";
+
+ String middle = "ms.schema";
+
+ String low = "ms";
+
+ String[] keys = {high, middle, low};
+
+ @InjectProperties(prefix = "root")
+ public static class ConfigWithAnnotation {
+ @InjectProperty(prefix = "override", keys = {"high", "low"})
+ public String strValue;
+ }
+
+ @Before
+ public void setup() {
+ // avoid write too many logs
+ Logger.getRootLogger().setLevel(Level.OFF);
+
+ ArchaiusUtils.resetConfig();
+
+ Logger.getRootLogger().setLevel(Level.INFO);
+ }
+
+ @After
+ public void teardown() {
+ ArchaiusUtils.resetConfig();
+ }
+
+ private void waitKeyForGC(PriorityPropertyManager priorityPropertyManager) {
+ long maxTime = 10000;
+ long currentTime = System.currentTimeMillis();
+ while (System.currentTimeMillis() - currentTime < maxTime) {
+ if (priorityPropertyManager.getPriorityPropertySet().isEmpty()
+ && priorityPropertyManager.getConfigObjectMap().isEmpty()) {
+ break;
+ }
+ System.runFinalization();
+ System.gc();
+ try {
+ Thread.yield();
+ Thread.sleep(10);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ }
+ }
+
+ @Test
+ public void testConfigurationsAreGCCollected() {
+ long timeBegin = System.currentTimeMillis();
+
+ PriorityPropertyManager priorityPropertyManager = new PriorityPropertyManager();
+ for (int i = 0; i < 100; i++) {
+ for (int j = 0; j < 100; j++) {
+ TestConfigObjectFactory.ConfigWithAnnotation configConfigObject = priorityPropertyManager.createConfigObject(
+ TestConfigObjectFactory.ConfigWithAnnotation.class);
+ Assert.assertEquals("abc", configConfigObject.strDef);
+ PriorityProperty<Long> configPriorityProperty = priorityPropertyManager.
+ createPriorityProperty(Long.class, -1L, -2L, keys);
+ Assert.assertEquals(-2L, (long) configPriorityProperty.getValue());
+ }
+ }
+
+ waitKeyForGC(priorityPropertyManager);
+
+ Assert.assertTrue(priorityPropertyManager.getPriorityPropertySet().isEmpty());
+ Assert.assertTrue(priorityPropertyManager.getConfigObjectMap().isEmpty());
+ for (DynamicProperty property : ConfigUtil.getAllDynamicProperties().values()) {
+ Assert.assertTrue(ConfigUtil.getCallbacks(property).isEmpty());
+ }
+
+ System.out.println("Token : " + (System.currentTimeMillis() - timeBegin));
+ }
+}
diff --git a/inspector/src/main/java/org/apache/servicecomb/inspector/internal/InspectorBootListener.java b/inspector/src/main/java/org/apache/servicecomb/inspector/internal/InspectorBootListener.java
index f5ec8c5..858c644 100644
--- a/inspector/src/main/java/org/apache/servicecomb/inspector/internal/InspectorBootListener.java
+++ b/inspector/src/main/java/org/apache/servicecomb/inspector/internal/InspectorBootListener.java
@@ -24,8 +24,6 @@ import org.slf4j.LoggerFactory;
public class InspectorBootListener implements BootListener {
private static final Logger LOGGER = LoggerFactory.getLogger(InspectorBootListener.class);
- private InspectorConfig inspectorConfig;
-
@Override
public int getOrder() {
return Short.MAX_VALUE;
@@ -33,7 +31,8 @@ public class InspectorBootListener implements BootListener {
@Override
public void onAfterTransport(BootEvent event) {
- inspectorConfig = event.getScbEngine().getPriorityPropertyManager().createConfigObject(InspectorConfig.class);
+ InspectorConfig inspectorConfig = event.getScbEngine().getPriorityPropertyManager()
+ .createConfigObject(InspectorConfig.class);
if (!inspectorConfig.isEnabled()) {
LOGGER.info("inspector is not enabled.");
return;
@@ -46,12 +45,4 @@ public class InspectorBootListener implements BootListener {
inspector.setPriorityPropertyManager(event.getScbEngine().getPriorityPropertyManager());
event.getScbEngine().getProducerProviderManager().registerSchema("inspector", inspector);
}
-
- @Override
- public void onAfterClose(BootEvent event) {
- // some UT case, inspectorConfig is null
- if (inspectorConfig != null) {
- event.getScbEngine().getPriorityPropertyManager().unregisterConfigObject(inspectorConfig);
- }
- }
}
diff --git a/inspector/src/main/java/org/apache/servicecomb/inspector/internal/InspectorImpl.java b/inspector/src/main/java/org/apache/servicecomb/inspector/internal/InspectorImpl.java
index 6e87be2..a747f2c 100644
--- a/inspector/src/main/java/org/apache/servicecomb/inspector/internal/InspectorImpl.java
+++ b/inspector/src/main/java/org/apache/servicecomb/inspector/internal/InspectorImpl.java
@@ -314,7 +314,7 @@ public class InspectorImpl {
priorityPropertyManager.getConfigObjectMap().values().stream()
.flatMap(Collection::stream)
.forEach(p -> views.add(createPriorityPropertyView(p)));
- priorityPropertyManager.getPriorityPropertyMap().keySet().forEach(p ->
+ priorityPropertyManager.getPriorityPropertySet().forEach(p ->
views.add(createPriorityPropertyView(p)));
return views;
}
diff --git a/inspector/src/test/java/org/apache/servicecomb/inspector/internal/TestInspectorBootListener.java b/inspector/src/test/java/org/apache/servicecomb/inspector/internal/TestInspectorBootListener.java
index 9b5df43..5bf1d73 100644
--- a/inspector/src/test/java/org/apache/servicecomb/inspector/internal/TestInspectorBootListener.java
+++ b/inspector/src/test/java/org/apache/servicecomb/inspector/internal/TestInspectorBootListener.java
@@ -43,7 +43,6 @@ public class TestInspectorBootListener {
@AfterClass
public static void teardown() {
- priorityPropertyManager.unregisterConfigObject(inspectorConfig);
ArchaiusUtils.resetConfig();
}
diff --git a/inspector/src/test/java/org/apache/servicecomb/inspector/internal/TestInspectorImpl.java b/inspector/src/test/java/org/apache/servicecomb/inspector/internal/TestInspectorImpl.java
index 172905f..a3ea9c4 100644
--- a/inspector/src/test/java/org/apache/servicecomb/inspector/internal/TestInspectorImpl.java
+++ b/inspector/src/test/java/org/apache/servicecomb/inspector/internal/TestInspectorImpl.java
@@ -362,10 +362,6 @@ public class TestInspectorImpl {
views.get(0).getDynamicProperties().stream().map(DynamicPropertyView::getKey).collect(Collectors.toList()),
Matchers.contains("high", "low"));
- priorityPropertyManager.unregisterPriorityProperty(priorityProperty);
- views = inspector.priorityProperties();
- Assert.assertTrue(views.isEmpty());
-
priorityPropertyManager.close();
inspector.setPriorityPropertyManager(null);
}
@@ -376,7 +372,5 @@ public class TestInspectorImpl {
Map<String, String> schemas = Deencapsulation.getField(inspector, "schemas");
Assert.assertTrue(schemas.get("schema1").indexOf("/webroot/rest/metrics") > 0);
-
- inspector.getScbEngine().getPriorityPropertyManager().unregisterConfigObject(inspector.getInspectorConfig());
}
}