Ticket #4626: 4626.patch

src/org/openstreetmap/josm/data/validation/ValidationTask.java

@@ -3 +3 @@
 
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.preferences.sources.ValidatorPrefHelper;
+import org.openstreetmap.josm.data.validation.tests.MapCSSTagChecker;
 import org.openstreetmap.josm.gui.MainApplication;
 import org.openstreetmap.josm.gui.MapFrame;
 import org.openstreetmap.josm.gui.PleaseWaitRunnable;
@@ -10 +11 @@
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
 import org.openstreetmap.josm.gui.progress.swing.PleaseWaitProgressMonitor;
 import org.openstreetmap.josm.gui.util.GuiHelper;
+import org.openstreetmap.josm.tools.Logging;
+import org.openstreetmap.josm.tools.Utils;
 
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ForkJoinPool;
+import java.util.concurrent.Future;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Consumer;
+import java.util.stream.Collectors;
 
 import static org.openstreetmap.josm.tools.I18n.tr;
 
@@ -21 +30 @@
  * Asynchronous task for running a collection of tests against a collection of primitives
  */
 public class ValidationTask extends PleaseWaitRunnable {
-    private Collection<Test> tests;
+    private final ArrayList<Test> tests;
     private final Collection<OsmPrimitive> validatedPrimitives;
     private final Collection<OsmPrimitive> formerValidatedPrimitives;
     private boolean canceled;
     private List<TestError> errors;
+    private static final ForkJoinPool THREAD_POOL = Utils.newForkJoinPool(
+            ValidatorPrefHelper.PREFIX + ".numberOfThreads", "validator-%d", Thread.NORM_PRIORITY);
 
     /**
      * Constructs a new {@code ValidationTask}
@@ -47 +58 @@
         super(tr("Validating"), progressMonitor, false /*don't ignore exceptions */);
         this.validatedPrimitives = validatedPrimitives;
         this.formerValidatedPrimitives = formerValidatedPrimitives;
-        this.tests = tests;
+        this.tests = new ArrayList<>(tests);
     }
 
     @Override
@@ -77 +88 @@
         if (tests == null || tests.isEmpty())
             return;
         errors = new ArrayList<>();
-        getProgressMonitor().setTicksCount(tests.size() * validatedPrimitives.size());
-        int testCounter = 0;
-        for (Test test : tests) {
-            if (canceled)
-                return;
-            testCounter++;
-            getProgressMonitor().setCustomText(tr("Test {0}/{1}: Starting {2}", testCounter, tests.size(), test.getName()));
+
+        AtomicInteger testCounter = new AtomicInteger();
+        runParallel(test -> {
             test.setBeforeUpload(false);
             test.setPartialSelection(formerValidatedPrimitives != null);
-            test.startTest(getProgressMonitor().createSubTaskMonitor(validatedPrimitives.size(), false));
-            test.visit(validatedPrimitives);
-            test.endTest();
-            errors.addAll(test.getErrors());
-            test.clear();
-        }
-        tests = null;
+            test.startTest(null);
+        });
+        runParallel(test -> {
+            getProgressMonitor().setCustomText(tr("Test {0}/{1}: Starting {2}", testCounter.incrementAndGet(), tests.size(), test.getName()));
+            if (test instanceof MapCSSTagChecker) {
+                // VARIANT 1
+                validatedPrimitives.parallelStream()
+                        .filter(test::isPrimitiveUsable)
+                        .forEach(p -> p.accept(test));
+            } else {
+                test.visit(validatedPrimitives);
+            }
+        });
+        runParallel(Test::endTest);
+        errors = tests.stream().flatMap(test -> test.getErrors().stream()).collect(Collectors.toList());
+
+        runParallel(Test::clear);
+        tests.clear();
+        tests.trimToSize();
+
         if (Boolean.TRUE.equals(ValidatorPrefHelper.PREF_USE_IGNORE.get())) {
             getProgressMonitor().setCustomText("");
             getProgressMonitor().subTask(tr("Updating ignored errors ..."));
@@ -103 +123 @@
         }
     }
 
+    private void runParallel(Consumer<Test> testConsumer) {
+        try {
+            Future<?> future = THREAD_POOL.submit(() -> tests.parallelStream().forEach(testConsumer));
+            future.get();
+        } catch (InterruptedException | ExecutionException e) {
+            if (e.getCause() instanceof RuntimeException) {
+                throw ((RuntimeException) e.getCause());
+            }
+            Logging.warn(e);
+            Thread.currentThread().interrupt();
+        }
+    }
+
     /**
      * Gets the validation errors accumulated until this moment.
      * @return The list of errors

src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java

@@ -238 +238 @@
      */
     @Override
     public void check(OsmPrimitive p) {
-        for (TestError e : getErrorsForPrimitive(p, ValidatorPrefHelper.PREF_OTHER.get())) {
-            addIfNotSimilar(e, errors);
+        Collection<TestError> errorsForPrimitive = getErrorsForPrimitive(p, ValidatorPrefHelper.PREF_OTHER.get());
+        if (errorsForPrimitive.isEmpty()) {
+            return;
+        }
+        synchronized (errors) {
+            for (TestError e : errorsForPrimitive) {
+                addIfNotSimilar(e, errors);
+            }
         }
     }
 
@@ -382 +388 @@
         mpAreaCache.clear();
 
         Set<OsmPrimitive> surrounding = new HashSet<>();
-        for (Entry<String, Set<MapCSSTagCheckerRule>> entry : checks.entrySet()) {
+        // VARIANT 2
+        checks.entrySet().parallelStream().forEach(entry -> {
             if (isCanceled()) {
-                break;
+                return;
             }
             if (urlPredicate != null && !urlPredicate.test(entry.getKey())) {
-                continue;
+                return;
             }
             visit(entry.getKey(), entry.getValue(), selection, surrounding);
-        }
+        });
     }
 
     /**
@@ -464 +471 @@
             if (tested.contains(p))
                 continue;
             Collection<TestError> additionalErrors = getErrorsForPrimitive(p, includeOtherSeverity);
-            for (TestError e : additionalErrors) {
-                if (e.getPrimitives().stream().anyMatch(tested::contains))
-                    addIfNotSimilar(e, errors);
+            if (additionalErrors.isEmpty())
+                continue;
+            synchronized (errors) {
+                for (TestError e : additionalErrors) {
+                    if (e.getPrimitives().stream().anyMatch(tested::contains)) {
+                        addIfNotSimilar(e, errors);
+                    }
+                }
             }
         }