zrlw commented on code in PR #16231:
URL: https://github.com/apache/dubbo/pull/16231#discussion_r3144607837
##########
dubbo-common/src/main/java/org/apache/dubbo/common/extension/ExtensionLoader.java:
##########
@@ -1027,7 +1027,7 @@ private Map<String, Class<?>> loadExtensionClasses()
throws InterruptedException
}
if (logger.isDebugEnabled()) {
- Long costMillis = startNanos != -1 ? (System.nanoTime() -
startNanos) / 1_000_000L : null;
+ Long costMillis = (System.nanoTime() - startNanos) / 1_000_000L;
Review Comment:
Capturing ```boolean debug = logger.isDebugEnabled();``` at the beginning of
methods and only computing/using timing when debug is true.
##########
dubbo-common/src/test/java/org/apache/dubbo/common/extension/ExtensionLoaderTest.java:
##########
@@ -882,4 +907,88 @@ public int getPriority() {
return MAX_PRIORITY;
}
}
+
+ private <T> ExtensionLoaderTestContext<T>
createExtensionLoaderTestContext(Class<T> type) {
+ FrameworkModel frameworkModel = new FrameworkModel();
+ ApplicationModel applicationModel = frameworkModel.newApplication();
+ return new ExtensionLoaderTestContext<>(
+ frameworkModel,
applicationModel.getExtensionDirector().getExtensionLoader(type));
+ }
+
+ private static final class ExtensionLoaderTestContext<T> implements
AutoCloseable {
+ private final FrameworkModel frameworkModel;
+ private final ExtensionLoader<T> extensionLoader;
+
+ private ExtensionLoaderTestContext(FrameworkModel frameworkModel,
ExtensionLoader<T> extensionLoader) {
+ this.frameworkModel = frameworkModel;
+ this.extensionLoader = extensionLoader;
+ }
+
+ @Override
+ public void close() {
+ frameworkModel.destroy();
+ }
+ }
+
+ private static final class LogCollector implements AutoCloseable {
+ private final LoggerContext context;
+ private final Configuration configuration;
+ private final String loggerName;
+ private final LoggerConfig loggerConfig;
+ private final TestAppender appender;
+
+ private LogCollector(
+ LoggerContext context,
+ Configuration configuration,
+ String loggerName,
+ LoggerConfig loggerConfig,
+ TestAppender appender) {
+ this.context = context;
+ this.configuration = configuration;
+ this.loggerName = loggerName;
+ this.loggerConfig = loggerConfig;
+ this.appender = appender;
+ }
+
+ static LogCollector attach(Class<?> loggerType) {
+ LoggerContext context = LoggerContext.getContext(false);
+ Configuration configuration = context.getConfiguration();
+ String loggerName = loggerType.getName();
+ TestAppender appender = new TestAppender("test-appender-" +
loggerType.getSimpleName());
+ appender.start();
+ configuration.addAppender(appender);
+
+ LoggerConfig loggerConfig = new LoggerConfig(loggerName,
org.apache.logging.log4j.Level.DEBUG, false);
+ loggerConfig.addAppender(appender,
org.apache.logging.log4j.Level.DEBUG, null);
+ configuration.addLogger(loggerName, loggerConfig);
+ context.updateLoggers();
+ return new LogCollector(context, configuration, loggerName,
loggerConfig, appender);
Review Comment:
it might cause trouble if we write more than one test classes for
ExtensionLoader.
##########
dubbo-common/src/main/java/org/apache/dubbo/common/extension/ExtensionLoader.java:
##########
@@ -795,15 +795,19 @@ private T createExtension(String name, boolean wrap) {
}
if (CollectionUtils.isNotEmpty(wrapperClassesList)) {
- appliedWrapperClasses = new
ArrayList<>(wrapperClassesList.size());
+ if (logger.isDebugEnabled()) {
+ appliedWrapperClasses = new
ArrayList<>(wrapperClassesList.size());
+ }
for (Class<?> wrapperClass : wrapperClassesList) {
Wrapper wrapper =
wrapperClass.getAnnotation(Wrapper.class);
boolean match = (wrapper == null)
|| ((ArrayUtils.isEmpty(wrapper.matches())
||
ArrayUtils.contains(wrapper.matches(), name))
&&
!ArrayUtils.contains(wrapper.mismatches(), name));
if (match) {
- appliedWrapperClasses.add(wrapperClass);
+ if (logger.isDebugEnabled()) {
Review Comment:
Capturing ```boolean debug = logger.isDebugEnabled();``` at the beginning of
methods
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
