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()); } }