remove unnecessary locking in parallel comparison strategy and enable it by default

jplag/src/main/java/de/jplag/JPlag.java

@@ -7,9 +7,6 @@ import de.jplag.clustering.ClusteringFactory;
 import de.jplag.exceptions.ExitException;
 import de.jplag.exceptions.SubmissionException;
 import de.jplag.options.JPlagOptions;
-import de.jplag.strategy.ComparisonMode;
-import de.jplag.strategy.ComparisonStrategy;
-import de.jplag.strategy.NormalComparisonStrategy;
 import de.jplag.strategy.ParallelComparisonStrategy;
 
 /**
@@ -22,7 +19,6 @@ public class JPlag {
 
     private final Language language;
     private final ComparisonStrategy comparisonStrategy;
-    private final GreedyStringTiling coreAlgorithm; // Contains the comparison logic.
 
     /**
      * Creates and initializes a JPlag instance, parameterized by a set of options.
@@ -30,9 +26,9 @@ public class JPlag {
      */
     public JPlag(JPlagOptions options) {
         this.options = options;
-        coreAlgorithm = new GreedyStringTiling(options);
         language = this.options.language();
-        comparisonStrategy = initializeComparisonStrategy(options.comparisonMode());
+        GreedyStringTiling coreAlgorithm = new GreedyStringTiling(options);
+        comparisonStrategy = new ParallelComparisonStrategy(options, coreAlgorithm);
     }
 
     /**
@@ -59,11 +55,4 @@ public class JPlag {
 
         return result;
     }
-
-    private ComparisonStrategy initializeComparisonStrategy(final ComparisonMode comparisonMode) {
-        return switch (comparisonMode) {
-            case NORMAL -> new NormalComparisonStrategy(options, coreAlgorithm);
-            case PARALLEL -> new ParallelComparisonStrategy(options, coreAlgorithm);
-        };
-    }
 }

jplag/src/main/java/de/jplag/strategy/AbstractComparisonStrategy.java

@@ -44,7 +44,7 @@ public abstract class AbstractComparisonStrategy implements ComparisonStrategy {
     /**
      * Compares two submissions and optionally returns the results if similarity is high enough.
      */
-    protected Optional<JPlagComparison> compareSubmissions(Submission first, Submission second, boolean withBaseCode) {
+    protected Optional<JPlagComparison> compareSubmissions(Submission first, Submission second) {
         JPlagComparison comparison = greedyStringTiling.compare(first, second);
         logger.info("Comparing {}-{}: {}", first.getName(), second.getName(), comparison.similarity());
 

jplag/src/main/java/de/jplag/strategy/NormalComparisonStrategy.java

@@ -1,38 +0,0 @@
-package de.jplag.strategy;
-
-import java.util.ArrayList;
-import java.util.List;
-
-import de.jplag.GreedyStringTiling;
-import de.jplag.JPlagComparison;
-import de.jplag.JPlagResult;
-import de.jplag.Submission;
-import de.jplag.SubmissionSet;
-import de.jplag.options.JPlagOptions;
-
-public class NormalComparisonStrategy extends AbstractComparisonStrategy {
-
-    public NormalComparisonStrategy(JPlagOptions options, GreedyStringTiling greedyStringTiling) {
-        super(options, greedyStringTiling);
-    }
-
-    @Override
-    public JPlagResult compareSubmissions(SubmissionSet submissionSet) {
-        boolean withBaseCode = submissionSet.hasBaseCode();
-        if (withBaseCode) {
-            compareSubmissionsToBaseCode(submissionSet);
-        }
-
-        List<Submission> submissions = submissionSet.getSubmissions();
-        long timeBeforeStartInMillis = System.currentTimeMillis();
-        List<JPlagComparison> comparisons = new ArrayList<>();
-
-        for (SubmissionTuple tuple : buildComparisonTuples(submissions)) {
-            compareSubmissions(tuple.left(), tuple.right(), withBaseCode).ifPresent(comparisons::add);
-        }
-
-        long durationInMillis = System.currentTimeMillis() - timeBeforeStartInMillis;
-        return new JPlagResult(comparisons, submissionSet, durationInMillis, options);
-    }
-
-}

jplag/src/main/java/de/jplag/strategy/ParallelComparisonStrategy.java

@@ -5,7 +5,6 @@ import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.*;
 import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -24,7 +23,6 @@ public class ParallelComparisonStrategy extends AbstractComparisonStrategy {
     private final ConcurrentMap<String, Lock> submissionLocks;
     private ExecutorService threadPool;
     private final List<JPlagComparison> comparisons;
-    private int successfulComparisons;
 
     public ParallelComparisonStrategy(JPlagOptions options, GreedyStringTiling greedyStringTiling) {
         super(options, greedyStringTiling);
@@ -43,24 +41,13 @@ public class ParallelComparisonStrategy extends AbstractComparisonStrategy {
         threadPool = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors());
         comparisons.clear();
         submissionLocks.clear();
-        successfulComparisons = 0;
 
         // Parallel compare:
         List<Submission> submissions = submissionSet.getSubmissions();
         List<SubmissionTuple> tuples = buildComparisonTuples(submissions);
         Collections.shuffle(tuples); // Reduces how often submission pairs must be re-submitted
         for (SubmissionTuple tuple : tuples) {
-            threadPool.execute(compareTuple(tuple, withBaseCode));
-        }
-
-        // Ensure termination:
-        while (successfulComparisons < tuples.size()) {
-            try {
-                Thread.sleep(5);
-            } catch (InterruptedException e) {
-                logger.error(e.getMessage(), e);
-                Thread.currentThread().interrupt();
-            }
+            threadPool.execute(compareTuple(tuple));
         }
 
         // Clean up and return result:
@@ -70,48 +57,19 @@ public class ParallelComparisonStrategy extends AbstractComparisonStrategy {
     }
 
     /**
-     * Creates a runnable which compares a submission tuple. If the submissions are locked, the runnable is re-submitted.
+     * Creates a runnable which compares a submission tuple.
      * @param tuple contains the submissions to compare.
-     * @param withBaseCode specifies if base code is used.
      * @return the runnable for parallel use.
      */
-    private Runnable compareTuple(SubmissionTuple tuple, boolean withBaseCode) {
+    private Runnable compareTuple(SubmissionTuple tuple) {
         return new Runnable() {
             @Override
             public void run() {
-                Lock leftLock = getOrCreateLock(tuple.left().getName());
-                Lock rightLock = getOrCreateLock(tuple.right().getName());
-                boolean hasLeft = leftLock.tryLock();
-                boolean hasRight = hasLeft && rightLock.tryLock();
-                try {
-                    if (hasLeft && hasRight) { // both locks acquired!
-                        compareSubmissions(tuple.left(), tuple.right(), withBaseCode).ifPresent(comparisons::add);
-                        synchronized (this) {
-                            successfulComparisons++;
-                        }
-                    } else {
-                        threadPool.execute(this); // re-submit runnable, as at least one submission is locked.
-                    }
-                } finally {
-                    if (hasRight) {
-                        rightLock.unlock();
-                    }
-                    if (hasLeft) {
-                        leftLock.unlock();
-                    }
-                }
+                compareSubmissions(tuple.left(), tuple.right()).ifPresent(comparisons::add);
             }
         };
     }
 
-    /**
-     * @return a lock for a given key. If it does not exist, it is created in a thread-safe manner.
-     */
-    private Lock getOrCreateLock(String key) {
-        submissionLocks.putIfAbsent(key, new ReentrantLock()); // atomic operation
-        return submissionLocks.get(key);
-    }
-
     /**
      * Shuts down the thread pool and awaits termination
      */