Skip to content

Commit

Permalink
Remove logging from the EMF Compare core
Browse files Browse the repository at this point in the history 
Most of these logs were only start/end information for comparison
phases. This depends on an external logger as the core is standalone.
Log4J 2 does not seem viable for eclipse as it requires an external
configuration file instead of being configurable through API, and we
can't have a global eclipse configuration just for compare. We wish to
remove the dependency towards Log4j 1.

Bug: 578953
Change-Id: Ib421d2f9487f84d8867f936faa0fc068e72e75d3
  • Loading branch information
@Kellindil
Kellindil committed Mar 22, 2022
1 parent 79dc341 commit 70d1945
Show file tree
Hide file tree
Showing 11 changed files with 1 addition and 307 deletions.
3 changes: 1 addition & 2 deletions plugins/org.eclipse.emf.compare/META-INF/MANIFEST.MF
View file
Expand Up @@ -43,5 +43,4 @@ Bundle-ActivationPolicy: lazy
Import-Package: com.google.common.base;version="[27.0.0,30.2.0)",
com.google.common.cache;version="[27.0.0,30.2.0)",
com.google.common.collect;version="[27.0.0,30.2.0)",
com.google.common.eventbus;version="[27.0.0,30.2.0)",
org.apache.log4j;version="[1.2.15,2.0.0)"
com.google.common.eventbus;version="[27.0.0,30.2.0)"
134 changes: 0 additions & 134 deletions plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/EMFCompare.java
View file
Expand Up @@ -16,14 +16,12 @@
import java.util.Iterator;
import java.util.List;

import org.apache.log4j.Logger;
import org.eclipse.emf.common.notify.Adapter;
import org.eclipse.emf.common.notify.Notifier;
import org.eclipse.emf.common.util.BasicDiagnostic;
import org.eclipse.emf.common.util.BasicMonitor;
import org.eclipse.emf.common.util.Diagnostic;
import org.eclipse.emf.common.util.DiagnosticChain;
import org.eclipse.emf.common.util.EList;
import org.eclipse.emf.common.util.Monitor;
import org.eclipse.emf.compare.conflict.IConflictDetector;
import org.eclipse.emf.compare.conflict.MatchBasedConflictDetector;
Expand Down Expand Up @@ -70,15 +68,6 @@ public class EMFCompare {
*/
public static final String DIAGNOSTIC_SOURCE = "org.eclipse.emf.compare"; //$NON-NLS-1$

/** Constant for logging. */
private static final String START = " - START"; //$NON-NLS-1$

/** Constant for logging. */
private static final String FINISH = " - FINISH"; //$NON-NLS-1$

/** The logger. */
private static final Logger LOGGER = Logger.getLogger(EMFCompare.class);

/** The registry we'll use to create a match engine for this comparison. */
private final IMatchEngine.Factory.Registry matchEngineFactoryRegistry;

Expand Down Expand Up @@ -204,19 +193,9 @@ public Comparison compare(IComparisonScope scope, final Monitor monitor) {
checkNotNull(scope);
checkNotNull(monitor);

// Used to compute the time spent in the method
long startTime = System.currentTimeMillis();

if (LOGGER.isInfoEnabled()) {
LOGGER.info("compare() - START"); //$NON-NLS-1$
}

Comparison comparison = null;
try {
Monitor subMonitor = new SafeSubMonitor(monitor);
if (LOGGER.isInfoEnabled()) {
LOGGER.info("compare() - starting step: MATCH"); //$NON-NLS-1$
}
comparison = matchEngineFactoryRegistry.getHighestRankingMatchEngineFactory(scope)
.getMatchEngine().match(scope, subMonitor);

Expand All @@ -226,72 +205,37 @@ public Comparison compare(IComparisonScope scope, final Monitor monitor) {
List<IPostProcessor> postProcessors = postProcessorDescriptorRegistry.getPostProcessors(scope);

// CHECKSTYLE:OFF Yes, I want to have ifs here and no constant for "post-processor".
if (LOGGER.isInfoEnabled()) {
LOGGER.info("compare() - starting step: POST-MATCH with " + postProcessors.size() //$NON-NLS-1$
+ " post-processors"); //$NON-NLS-1$
}
postMatch(comparison, postProcessors, subMonitor);
monitor.worked(1);

if (!hasToStop(comparison, monitor)) {
if (LOGGER.isInfoEnabled()) {
LOGGER.info("compare() - starting step: DIFF"); //$NON-NLS-1$
}
diffEngine.diff(comparison, subMonitor);
monitor.worked(1);
if (LOGGER.isInfoEnabled()) {
LOGGER.info("compare() - starting step: POST-DIFF with " //$NON-NLS-1$
+ postProcessors.size() + " post-processors"); //$NON-NLS-1$
}
postDiff(comparison, postProcessors, subMonitor);
monitor.worked(1);

if (!hasToStop(comparison, monitor)) {
if (LOGGER.isInfoEnabled()) {
LOGGER.info("compare() - starting step: REQUIREMENTS"); //$NON-NLS-1$
}
reqEngine.computeRequirements(comparison, subMonitor);
monitor.worked(1);
if (LOGGER.isInfoEnabled()) {
LOGGER.info("compare() - starting step: POST-REQUIREMENTS with " //$NON-NLS-1$
+ postProcessors.size() + " post-processors"); //$NON-NLS-1$
}
postRequirements(comparison, postProcessors, subMonitor);
monitor.worked(1);

if (!hasToStop(comparison, monitor)) {
if (LOGGER.isInfoEnabled()) {
LOGGER.info("compare() - starting step: EQUIVALENCES"); //$NON-NLS-1$
}
equiEngine.computeEquivalences(comparison, subMonitor);
monitor.worked(1);
if (LOGGER.isInfoEnabled()) {
LOGGER.info("compare() - starting step: POST-EQUIVALENCES with " //$NON-NLS-1$
+ postProcessors.size() + " post-processors"); //$NON-NLS-1$
}
postEquivalences(comparison, postProcessors, subMonitor);
monitor.worked(1);

if (LOGGER.isInfoEnabled()) {
LOGGER.info("compare() - starting step: CONFLICT"); //$NON-NLS-1$
}
detectConflicts(comparison, postProcessors, subMonitor);
monitor.worked(1);

if (LOGGER.isInfoEnabled()) {
LOGGER.info("compare() - starting step: POST-COMPARISON with " //$NON-NLS-1$
+ postProcessors.size() + " post-processors"); //$NON-NLS-1$
}
// CHECKSTYLE:ON
postComparison(comparison, postProcessors, subMonitor);
monitor.worked(1);
}
}
}
} catch (ComparisonCanceledException e) {
if (LOGGER.isInfoEnabled()) {
LOGGER.info("compare() - Comparison has been canceled"); //$NON-NLS-1$
}
if (comparison == null) {
comparison = new ComparisonSpec();
}
Expand All @@ -307,10 +251,6 @@ public Comparison compare(IComparisonScope scope, final Monitor monitor) {
monitor.done();
}

if (LOGGER.isInfoEnabled()) {
logEndOfComparison(comparison, startTime);
}

// Add scope to the comparison's adapters to make it available throughout the framework
if (scope instanceof Adapter) {
comparison.eAdapters().add((Adapter)scope);
Expand Down Expand Up @@ -352,32 +292,6 @@ private void installResourceChangeAdapter(Comparison comparison, IComparisonScop
}
}

/**
* Log useful informations at the end of the comparison process.
*
* @param comparison
* The comparison
* @param start
* The time when the method start
*/
private void logEndOfComparison(Comparison comparison, long start) {
long duration = System.currentTimeMillis() - start;
int diffQuantity = comparison.getDifferences().size();
int conflictQuantity = comparison.getConflicts().size();
int matchQuantity = 0;
EList<Match> matches = comparison.getMatches();
for (Match match : matches) {
matchQuantity++;
Iterator<Match> subMatchIterator = match.getAllSubmatches().iterator();
while (subMatchIterator.hasNext()) {
matchQuantity++;
subMatchIterator.next();
}
}
LOGGER.info("compare() - FINISH - " + matchQuantity + " matches, " + diffQuantity + " diffs and " //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+ conflictQuantity + " conflicts found in " + duration + "ms"); //$NON-NLS-1$ //$NON-NLS-2$
}

/**
* Launches the conflict detection engine, if any has been given and if the comparison is three way.
*
Expand Down Expand Up @@ -411,15 +325,7 @@ private void postMatch(final Comparison comparison, List<IPostProcessor> postPro
Iterator<IPostProcessor> processorsIterator = postProcessors.iterator();
while (!hasToStop(comparison, monitor) && processorsIterator.hasNext()) {
final IPostProcessor iPostProcessor = processorsIterator.next();
if (LOGGER.isInfoEnabled()) {
LOGGER.info("postMatch with post-processor: " //$NON-NLS-1$
+ iPostProcessor.getClass().getName() + START);
}
iPostProcessor.postMatch(comparison, monitor);
if (LOGGER.isInfoEnabled()) {
LOGGER.info("postMatch with post-processor: " //$NON-NLS-1$
+ iPostProcessor.getClass().getName() + FINISH);
}
}
}

Expand All @@ -438,15 +344,7 @@ private void postDiff(final Comparison comparison, List<IPostProcessor> postProc
Iterator<IPostProcessor> processorsIterator = postProcessors.iterator();
while (!hasToStop(comparison, monitor) && processorsIterator.hasNext()) {
final IPostProcessor iPostProcessor = processorsIterator.next();
if (LOGGER.isInfoEnabled()) {
LOGGER.info("postDiff with post-processor: " + iPostProcessor.getClass().getName() //$NON-NLS-1$
+ START);
}
iPostProcessor.postDiff(comparison, monitor);
if (LOGGER.isInfoEnabled()) {
LOGGER.info("postDiff with post-processor: " + iPostProcessor.getClass().getName() //$NON-NLS-1$
+ FINISH);
}
}
}

Expand All @@ -465,15 +363,7 @@ private void postRequirements(final Comparison comparison, List<IPostProcessor>
Iterator<IPostProcessor> processorsIterator = postProcessors.iterator();
while (!hasToStop(comparison, monitor) && processorsIterator.hasNext()) {
final IPostProcessor iPostProcessor = processorsIterator.next();
if (LOGGER.isInfoEnabled()) {
LOGGER.info("postRequirements with post-processor: " //$NON-NLS-1$
+ iPostProcessor.getClass().getName() + START);
}
iPostProcessor.postRequirements(comparison, monitor);
if (LOGGER.isInfoEnabled()) {
LOGGER.info("postRequirements with post-processor: " //$NON-NLS-1$
+ iPostProcessor.getClass().getName() + FINISH);
}
}
}

Expand All @@ -492,15 +382,7 @@ private void postEquivalences(final Comparison comparison, List<IPostProcessor>
Iterator<IPostProcessor> processorsIterator = postProcessors.iterator();
while (!hasToStop(comparison, monitor) && processorsIterator.hasNext()) {
final IPostProcessor iPostProcessor = processorsIterator.next();
if (LOGGER.isInfoEnabled()) {
LOGGER.info("postEquivalences with post-processor: " //$NON-NLS-1$
+ iPostProcessor.getClass().getName() + START);
}
iPostProcessor.postEquivalences(comparison, monitor);
if (LOGGER.isInfoEnabled()) {
LOGGER.info("postEquivalences with post-processor: " //$NON-NLS-1$
+ iPostProcessor.getClass().getName() + FINISH);
}
}
}

Expand All @@ -519,15 +401,7 @@ private void postConflicts(final Comparison comparison, List<IPostProcessor> pos
Iterator<IPostProcessor> processorsIterator = postProcessors.iterator();
while (!hasToStop(comparison, monitor) && processorsIterator.hasNext()) {
final IPostProcessor iPostProcessor = processorsIterator.next();
if (LOGGER.isInfoEnabled()) {
LOGGER.info("postConflicts with post-processor: " //$NON-NLS-1$
+ iPostProcessor.getClass().getName() + START);
}
iPostProcessor.postConflicts(comparison, monitor);
if (LOGGER.isInfoEnabled()) {
LOGGER.info("postConflicts with post-processor: " //$NON-NLS-1$
+ iPostProcessor.getClass().getName() + FINISH);
}
}
}

Expand All @@ -547,19 +421,11 @@ private void postComparison(final Comparison comparison, List<IPostProcessor> po
int postProcessorIndex = 1;
while (!hasToStop(comparison, monitor) && processorsIterator.hasNext()) {
final IPostProcessor postProcessor = processorsIterator.next();
if (LOGGER.isInfoEnabled()) {
LOGGER.info("postComparison with post-processor: " //$NON-NLS-1$
+ postProcessor.getClass().getName() + START);
}
monitor.subTask(EMFCompareMessages.getString("PostComparison.monitor.postprocessor", //$NON-NLS-1$
postProcessor.getClass().getSimpleName(), String.valueOf(postProcessorIndex),
String.valueOf(postProcessors.size())));
postProcessor.postComparison(comparison, monitor);
postProcessorIndex++;
if (LOGGER.isInfoEnabled()) {
LOGGER.info("postComparison with post-processor: " //$NON-NLS-1$
+ postProcessor.getClass().getName() + FINISH);
}
}
}

Expand Down
13 changes: 0 additions & 13 deletions ...org.eclipse.emf.compare/src/org/eclipse/emf/compare/conflict/DefaultConflictDetector.java
View file
Expand Up @@ -24,7 +24,6 @@
import java.util.stream.Stream;
import java.util.stream.StreamSupport;

import org.apache.log4j.Logger;
import org.eclipse.emf.common.util.Monitor;
import org.eclipse.emf.compare.AttributeChange;
import org.eclipse.emf.compare.CompareFactory;
Expand Down Expand Up @@ -69,20 +68,13 @@
*/
public class DefaultConflictDetector implements IConflictDetector {

/** The logger. */
private static final Logger LOGGER = Logger.getLogger(DefaultConflictDetector.class);

/**
* {@inheritDoc}
*
* @see org.eclipse.emf.compare.conflict.IConflictDetector#detect(org.eclipse.emf.compare.Comparison,
* org.eclipse.emf.common.util.Monitor)
*/
public void detect(Comparison comparison, Monitor monitor) {
long start = System.currentTimeMillis();
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("detect conflicts - START"); //$NON-NLS-1$
}
final List<Diff> differences = comparison.getDifferences();
final int diffCount = differences.size();

Expand All @@ -99,11 +91,6 @@ public void detect(Comparison comparison, Monitor monitor) {
Stream<Diff> conflictCandidates = differences.stream().filter(possiblyConflictingWith(diff));
checkConflict(comparison, diff, conflictCandidates::iterator);
}

if (LOGGER.isInfoEnabled()) {
LOGGER.info(String.format("detect conflicts - END - Took %d ms", Long.valueOf(System //$NON-NLS-1$
.currentTimeMillis() - start)));
}
}

/**
Expand Down
13 changes: 0 additions & 13 deletions ....eclipse.emf.compare/src/org/eclipse/emf/compare/conflict/MatchBasedConflictDetector.java
View file
Expand Up @@ -12,7 +12,6 @@

import java.util.List;

import org.apache.log4j.Logger;
import org.eclipse.emf.common.util.Monitor;
import org.eclipse.emf.compare.Comparison;
import org.eclipse.emf.compare.ComparisonCanceledException;
Expand All @@ -38,20 +37,13 @@
*/
public class MatchBasedConflictDetector implements IConflictDetector {

/** The logger. */
private static final Logger LOGGER = Logger.getLogger(MatchBasedConflictDetector.class);

/**
* {@inheritDoc}
*
* @see org.eclipse.emf.compare.conflict.IConflictDetector#detect(org.eclipse.emf.compare.Comparison,
* org.eclipse.emf.common.util.Monitor)
*/
public void detect(Comparison comparison, Monitor monitor) {
long start = System.currentTimeMillis();
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("detect conflicts - START"); //$NON-NLS-1$
}
final List<Diff> differences = comparison.getDifferences();
final int diffCount = differences.size();

Expand All @@ -68,10 +60,5 @@ public void detect(Comparison comparison, Monitor monitor) {
AbstractConflictSearch<? extends Diff> search = conflictSearchFactory.doSwitch(diff);
search.detectConflicts();
}

if (LOGGER.isInfoEnabled()) {
LOGGER.info(String.format("detect conflicts - END - Took %d ms", Long.valueOf(System //$NON-NLS-1$
.currentTimeMillis() - start)));
}
}
}
12 changes: 0 additions & 12 deletions plugins/org.eclipse.emf.compare/src/org/eclipse/emf/compare/diff/DefaultDiffEngine.java
View file
Expand Up @@ -20,7 +20,6 @@
import java.util.Iterator;
import java.util.List;

import org.apache.log4j.Logger;
import org.eclipse.emf.common.util.Monitor;
import org.eclipse.emf.compare.Comparison;
import org.eclipse.emf.compare.ComparisonCanceledException;
Expand Down Expand Up @@ -61,9 +60,6 @@ public class DefaultDiffEngine implements IDiffEngine {
*/
protected static final Object UNMATCHED_VALUE = new Object();

/** The logger. */
private static final Logger LOGGER = Logger.getLogger(DefaultDiffEngine.class);

/**
* The diff processor that will be used by this engine. Should be passed by the constructor and accessed
* by {@link #getDiffProcessor()}.
Expand Down Expand Up @@ -120,18 +116,10 @@ protected <E> int indexOf(Comparison comparison, List<E> list, E element) {
* org.eclipse.emf.common.util.Monitor)
*/
public void diff(Comparison comparison, Monitor monitor) {
long start = System.currentTimeMillis();
if (LOGGER.isDebugEnabled()) {
LOGGER.debug(String.format("detect differences - START")); //$NON-NLS-1$
}
monitor.subTask(EMFCompareMessages.getString("DefaultDiffEngine.monitor.diff")); //$NON-NLS-1$
for (Match rootMatch : comparison.getMatches()) {
checkForDifferences(rootMatch, monitor);
}
if (LOGGER.isInfoEnabled()) {
LOGGER.info(String.format("detect differences - END - Took %d ms", Long.valueOf(System //$NON-NLS-1$
.currentTimeMillis() - start)));
}
}

/**
Expand Down

0 comments on commit 70d1945

Please sign in to comment.