Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

[Scheduler] Dropped usage of SimpleDateFormat for trace logging #3508

Merged
merged 1 commit into from
May 26, 2017
+20 −20
Merged

[Scheduler] Dropped usage of SimpleDateFormat for trace logging #3508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 10 additions & 9 deletions ...smarthome.core/src/main/java/org/eclipse/smarthome/core/scheduler/AbstractExpression.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
package org.eclipse.smarthome.core.scheduler;

import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Calendar;
import java.util.Collections;
Expand All @@ -30,8 +29,6 @@ public abstract class AbstractExpression<E extends AbstractExpressionPart> imple

private final Logger logger = LoggerFactory.getLogger(this.getClass().getName());

static SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS");

private int minimumCandidates = 1;
private int maximumCandidates = 100;

Expand Down Expand Up @@ -95,7 +92,7 @@ public void setStartDate(Date startDate) throws IllegalArgumentException, ParseE
throw new IllegalArgumentException("The start date of the rule must not be null");
}
this.startDate = startDate;
logger.trace("Setting the start date to {}", sdf.format(startDate));
logger.trace("Setting the start date to {}", startDate);
parseExpression(expression);
}

Expand Down Expand Up @@ -185,8 +182,10 @@ public final void parseExpression(String expression, boolean searchMode)
continueSearch = false;
}

for (Date aDate : getCandidates()) {
logger.trace("Final candidate {} is {}", getCandidates().indexOf(aDate), sdf.format(aDate));
if (logger.isTraceEnabled()) {
for (Date aDate : getCandidates()) {
logger.trace("Final candidate {} is {}", getCandidates().indexOf(aDate), aDate);
}
}
}

Expand All @@ -197,9 +196,11 @@ protected void applyExpressionParts(boolean searchMode) {
for (ExpressionPart part : getExpressionParts()) {
logger.trace("Expanding {} from {} candidates", part.getClass().getSimpleName(), getCandidates().size());
setCandidates(part.apply(startDate, getCandidates()));
logger.trace("Expanded to {} candidates", getCandidates().size());
for (Date aDate : getCandidates()) {
logger.trace("Candidate {} is {}", getCandidates().indexOf(aDate), sdf.format(aDate));
if (logger.isTraceEnabled()) {
logger.trace("Expanded to {} candidates", getCandidates().size());
for (Date aDate : getCandidates()) {
logger.trace("Candidate {} is {}", getCandidates().indexOf(aDate), aDate);
}
}
if (searchMode) {
pruneFarthest();
Expand Down
21 changes: 10 additions & 11 deletions ....core/src/main/java/org/eclipse/smarthome/core/scheduler/ExpressionThreadPoolManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
*/
package org.eclipse.smarthome.core.scheduler;

import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
Expand Down Expand Up @@ -36,18 +35,14 @@
*/
public class ExpressionThreadPoolManager extends ThreadPoolManager {

private final static Logger logger = LoggerFactory.getLogger(ExpressionThreadPoolManager.class);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using logger for every inner class, you could also do this:

private final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

This will be automatically resolved for every class instance that uses it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you really sure that this will give a different logger class when calling the same instance from the different locations, I cannot see how that would work out. The lookup is done only once when initializing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually use this in inheritance pattern in which the logger is mentioned in interface or base (abstract) class so that the logger gets resolved for every child class instances. I completely overlooked in this scenario that it will be resolved only for the top-level class during instantiation and the same logger instance will be used by the inner classes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:-)


static SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS");

/**
* Returns an instance of an expression-driven scheduled thread pool service. If it is the first request for the
* given pool name, the instance is newly created.
*
* @param poolName a short name used to identify the pool, e.g. "discovery"
* @return an instance to use
*/
static public ExpressionThreadPoolExecutor getExpressionScheduledPool(String poolName) {
public static ExpressionThreadPoolExecutor getExpressionScheduledPool(String poolName) {
ExecutorService pool = pools.get(poolName);
if (pool == null) {
synchronized (pools) {
Expand All @@ -59,8 +54,8 @@ static public ExpressionThreadPoolExecutor getExpressionScheduledPool(String poo
((ThreadPoolExecutor) pool).setKeepAliveTime(THREAD_TIMEOUT, TimeUnit.SECONDS);
((ThreadPoolExecutor) pool).allowCoreThreadTimeOut(true);
pools.put(poolName, pool);
logger.debug("Created an expression-drive scheduled thread pool '{}' of size {}",
new Object[] { poolName, cfg });
LoggerFactory.getLogger(ExpressionThreadPoolManager.class)
.debug("Created an expression-drive scheduled thread pool '{}' of size {}", poolName, cfg);
}
}
}
Expand All @@ -73,6 +68,8 @@ static public ExpressionThreadPoolExecutor getExpressionScheduledPool(String poo

public static class ExpressionThreadPoolExecutor extends ScheduledThreadPoolExecutor {

private final Logger logger = LoggerFactory.getLogger(ExpressionThreadPoolExecutor.class);

private Map<Expression, Runnable> scheduled = new ConcurrentHashMap<>();
private Map<Runnable, ArrayList<Future<?>>> futures = Collections
.synchronizedMap(new HashMap<Runnable, ArrayList<Future<?>>>());
Expand All @@ -82,12 +79,14 @@ public static class ExpressionThreadPoolExecutor extends ScheduledThreadPoolExec

public ExpressionThreadPoolExecutor(final String poolName, int corePoolSize) {
this(poolName, corePoolSize, new NamedThreadFactory(poolName), new ThreadPoolExecutor.DiscardPolicy() {

private final Logger logger = LoggerFactory.getLogger(ThreadPoolExecutor.DiscardPolicy.class);

// The pool is bounded and rejections will happen during shutdown
@Override
public void rejectedExecution(Runnable runnable, ThreadPoolExecutor threadPoolExecutor) {
// Log and discard
logger.warn("Thread pool '{}' rejected execution of {}",
new Object[] { poolName, runnable.getClass() });
logger.debug("Thread pool '{}' rejected execution of {}", poolName, runnable.getClass());
super.rejectedExecution(runnable, threadPoolExecutor);
}
});
Expand Down Expand Up @@ -178,7 +177,7 @@ public void run() {
Date time = e.getTimeAfter(now);

if (time != null) {
logger.trace("Expression's '{}' next execution time is {}", e, sdf.format(time));
logger.trace("Expression's '{}' next execution time is {}", e, time);

Runnable task = scheduled.get(e);

Expand Down