vy commented on code in PR #2454:
URL: https://github.com/apache/logging-log4j2/pull/2454#discussion_r1663089137
##########
log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java:
##########
@@ -517,45 +532,64 @@ private Environment(final PropertySource propertySource) {
ServiceLoader.load(PropertySource.class,
PropertiesUtil.class.getClassLoader()),
LOGGER)
.forEach(sources::add);
+ sources.sort(Comparator.INSTANCE);
reload();
}
- /**
- * Allow a PropertySource to be added.
- * @param propertySource The PropertySource to add.
- */
- public void addPropertySource(final PropertySource propertySource) {
- sources.add(propertySource);
+ private void addPropertySource(final PropertySource propertySource) {
+ try {
+ sourceWriteLock.lock();
+ if (!sources.contains(propertySource)) {
+ sources.add(propertySource);
+ sources.sort(Comparator.INSTANCE);
Review Comment:
How often `reload()` is called? Is the performance difference in the big
picture notable enough to warrant the extra optimization? If not, I am inclined
to go with the simplest approach.
I am not a big fan on these kind of discussions. Please address this
conversation as you see fit, resolve it, and let's merge this PR.
--
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: notifications-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
