SONAR-5528 Improve performance of syntax highlighting API

6 files changed, 110 insertions, 77 deletions

diff --git a/sonar-batch/src/main/java/org/sonar/batch/highlighting/SyntaxHighlightingData.java b/sonar-batch/src/main/java/org/sonar/batch/highlighting/SyntaxHighlightingData.java
index 56c9b795ac9..8e4f5314ec6 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/highlighting/SyntaxHighlightingData.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/highlighting/SyntaxHighlightingData.java
@@ -21,29 +21,28 @@ package org.sonar.batch.highlighting;
 
 import org.sonar.batch.index.Data;
 
-import java.util.List;
+import java.util.ArrayList;
+import java.util.Collection;
 
 public class SyntaxHighlightingData implements Data {
 
   private static final String FIELD_SEPARATOR = ",";
   private static final String RULE_SEPARATOR = ";";
 
-  private List<SyntaxHighlightingRule> syntaxHighlightingRuleSet;
+  private Collection<SyntaxHighlightingRule> syntaxHighlightingRuleSet;
 
-  public SyntaxHighlightingData(List<SyntaxHighlightingRule> syntaxHighlightingRuleSet) {
-    this.syntaxHighlightingRuleSet = syntaxHighlightingRuleSet;
+  public SyntaxHighlightingData(Collection<SyntaxHighlightingRule> syntaxHighlightingRuleSet) {
+    this.syntaxHighlightingRuleSet = new ArrayList<SyntaxHighlightingRule>(syntaxHighlightingRuleSet);
   }
 
-  public List<SyntaxHighlightingRule> syntaxHighlightingRuleSet() {
+  public Collection<SyntaxHighlightingRule> syntaxHighlightingRuleSet() {
     return syntaxHighlightingRuleSet;
   }
 
   @Override
   public String writeString() {
     StringBuilder sb = new StringBuilder();
-    List<SyntaxHighlightingRule> orderedRules = syntaxHighlightingRuleSet;
-
-    for (SyntaxHighlightingRule highlightingRule : orderedRules) {
+    for (SyntaxHighlightingRule highlightingRule : syntaxHighlightingRuleSet) {
       sb.append(highlightingRule.getStartPosition())
         .append(FIELD_SEPARATOR)
         .append(highlightingRule.getEndPosition())
diff --git a/sonar-batch/src/main/java/org/sonar/batch/highlighting/SyntaxHighlightingDataBuilder.java b/sonar-batch/src/main/java/org/sonar/batch/highlighting/SyntaxHighlightingDataBuilder.java
index 3a74dbf4884..792cfb2eb3a 100644
--- a/sonar-batch/src/main/java/org/sonar/batch/highlighting/SyntaxHighlightingDataBuilder.java
+++ b/sonar-batch/src/main/java/org/sonar/batch/highlighting/SyntaxHighlightingDataBuilder.java
@@ -20,33 +20,38 @@
 package org.sonar.batch.highlighting;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Predicate;
-import com.google.common.collect.Collections2;
 import com.google.common.collect.Ordering;
-import org.slf4j.LoggerFactory;
+import org.elasticsearch.common.collect.Sets;
 
 import javax.annotation.Nullable;
 
-import java.util.Collection;
-import java.util.List;
-
-import static com.google.common.collect.Lists.newArrayList;
+import java.util.Iterator;
+import java.util.Set;
 
 public class SyntaxHighlightingDataBuilder {
 
-  private List<SyntaxHighlightingRule> syntaxHighlightingRuleSet;
+  private Set<SyntaxHighlightingRule> syntaxHighlightingRuleSet;
 
   public SyntaxHighlightingDataBuilder() {
-    syntaxHighlightingRuleSet = newArrayList();
+    syntaxHighlightingRuleSet = Sets.newTreeSet(new Ordering<SyntaxHighlightingRule>() {
+      @Override
+      public int compare(@Nullable SyntaxHighlightingRule left,
+        @Nullable SyntaxHighlightingRule right) {
+        int result = left.getStartPosition() - right.getStartPosition();
+        if (result == 0) {
+          result = left.getEndPosition() - right.getEndPosition();
+        }
+        return result;
+      }
+    });
+  }
+
+  @VisibleForTesting
+  public Set<SyntaxHighlightingRule> getSyntaxHighlightingRuleSet() {
+    return syntaxHighlightingRuleSet;
   }
 
   public SyntaxHighlightingDataBuilder registerHighlightingRule(int startOffset, int endOffset, String typeOfText) {
-    if (ruleConflictsWithExistingRules(startOffset, endOffset)) {
-      String errorMsg = String.format("Cannot register highlighting rule for characters from %s to %s as it " +
-        "overlaps at least one existing rule", startOffset, endOffset);
-      LoggerFactory.getLogger(SyntaxHighlightingDataBuilder.class).error(errorMsg);
-      throw new UnsupportedOperationException(errorMsg);
-    }
     SyntaxHighlightingRule syntaxHighlightingRule = SyntaxHighlightingRule.create(startOffset, endOffset,
       typeOfText);
     this.syntaxHighlightingRuleSet.add(syntaxHighlightingRule);
@@ -54,50 +59,24 @@ public class SyntaxHighlightingDataBuilder {
   }
 
   public SyntaxHighlightingData build() {
-    return new SyntaxHighlightingData(getSortedRules());
+    checkOverlappingBoudaries();
+    return new SyntaxHighlightingData(syntaxHighlightingRuleSet);
   }
 
-  private boolean ruleConflictsWithExistingRules(final int startOffset, final int endOffset) {
-    Collection<SyntaxHighlightingRule> conflictingRules = Collections2
-      .filter(syntaxHighlightingRuleSet, new Predicate<SyntaxHighlightingRule>() {
-        @Override
-        public boolean apply(@Nullable SyntaxHighlightingRule syntaxHighlightingRule) {
-
-          if (syntaxHighlightingRule != null) {
-            boolean overlapsStartBoundary = startOffset < syntaxHighlightingRule.getStartPosition()
-              && endOffset >= syntaxHighlightingRule.getStartPosition() + 1
-              && endOffset < syntaxHighlightingRule.getEndPosition();
-
-            boolean overlapsEndBoundary = startOffset > syntaxHighlightingRule.getStartPosition()
-              && startOffset < syntaxHighlightingRule.getEndPosition()
-              && endOffset > syntaxHighlightingRule.getEndPosition();
-
-            return overlapsStartBoundary || overlapsEndBoundary;
-          }
-          return false;
+  private void checkOverlappingBoudaries() {
+    if (syntaxHighlightingRuleSet.size() > 1) {
+      Iterator<SyntaxHighlightingRule> it = syntaxHighlightingRuleSet.iterator();
+      SyntaxHighlightingRule previous = it.next();
+      while (it.hasNext()) {
+        SyntaxHighlightingRule current = it.next();
+        if (previous.getEndPosition() > current.getStartPosition() && !(previous.getEndPosition() >= current.getEndPosition())) {
+          String errorMsg = String.format("Cannot register highlighting rule for characters from %s to %s as it " +
+            "overlaps at least one existing rule", current.getStartPosition(), current.getEndPosition());
+          throw new IllegalStateException(errorMsg);
         }
-      });
-    return !conflictingRules.isEmpty();
-  }
-
-  @VisibleForTesting
-  public List<SyntaxHighlightingRule> getSortedRules() {
-    Ordering<SyntaxHighlightingRule> ruleOrdering = new Ordering<SyntaxHighlightingRule>() {
-      @Override
-      public int compare(@Nullable SyntaxHighlightingRule left,
-        @Nullable SyntaxHighlightingRule right) {
-        int result;
-        if (left != null && right != null) {
-          result = left.getStartPosition() - right.getStartPosition();
-          if (result == 0) {
-            result = left.getEndPosition() - right.getEndPosition();
-          }
-          return result;
-        }
-        return left != null ? 1 : -1;
+        previous = current;
       }
-    };
-
-    return ruleOrdering.sortedCopy(syntaxHighlightingRuleSet);
+    }
   }
+
 }
diff --git a/sonar-batch/src/test/java/org/sonar/batch/highlighting/SyntaxHighlightingDataBuilderTest.java b/sonar-batch/src/test/java/org/sonar/batch/highlighting/SyntaxHighlightingDataBuilderTest.java
index 6fad264c831..e511bc388de 100644
--- a/sonar-batch/src/test/java/org/sonar/batch/highlighting/SyntaxHighlightingDataBuilderTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/highlighting/SyntaxHighlightingDataBuilderTest.java
@@ -19,20 +19,18 @@
  */
 package org.sonar.batch.highlighting;
 
-
-import org.sonar.batch.highlighting.SyntaxHighlightingDataBuilder;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 
-import java.util.List;
+import java.util.Collection;
 
 import static org.fest.assertions.Assertions.assertThat;
 
 public class SyntaxHighlightingDataBuilderTest {
 
-  private List<SyntaxHighlightingRule> highlightingRules;
+  private Collection<SyntaxHighlightingRule> highlightingRules;
 
   @Rule
   public ExpectedException throwable = ExpectedException.none();
@@ -48,7 +46,7 @@ public class SyntaxHighlightingDataBuilderTest {
     highlightingDataBuilder.registerHighlightingRule(24, 65, "cppd");
     highlightingDataBuilder.registerHighlightingRule(12, 20, "cd");
 
-    highlightingRules = highlightingDataBuilder.getSortedRules();
+    highlightingRules = highlightingDataBuilder.getSyntaxHighlightingRuleSet();
   }
 
   @Test
@@ -58,17 +56,27 @@ public class SyntaxHighlightingDataBuilderTest {
 
   @Test
   public void should_order_by_start_then_end_offset() throws Exception {
-    assertThat(highlightingRules).onProperty("startPosition").containsExactly(0, 10, 12, 24, 24, 42);
-    assertThat(highlightingRules).onProperty("endPosition").containsExactly(10, 12, 20, 38, 65, 50);
-    assertThat(highlightingRules).onProperty("textType").containsExactly("cd", "k", "cd", "k", "cppd", "k");
+    assertThat(highlightingRules).onProperty("startPosition").containsOnly(0, 10, 12, 24, 24, 42);
+    assertThat(highlightingRules).onProperty("endPosition").containsOnly(10, 12, 20, 38, 65, 50);
+    assertThat(highlightingRules).onProperty("textType").containsOnly("cd", "k", "cd", "k", "cppd", "k");
+  }
+
+  @Test
+  public void should_suport_overlapping() throws Exception {
+    SyntaxHighlightingDataBuilder builder = new SyntaxHighlightingDataBuilder();
+    builder.registerHighlightingRule(0, 15, "k");
+    builder.registerHighlightingRule(8, 12, "cppd");
+    builder.build();
   }
 
   @Test
-  public void should_prevent_rules_overlapping() throws Exception {
-    throwable.expect(UnsupportedOperationException.class);
+  public void should_prevent_boudaries_overlapping() throws Exception {
+    throwable.expect(IllegalStateException.class);
+    throwable.expectMessage("Cannot register highlighting rule for characters from 8 to 15 as it overlaps at least one existing rule");
 
     SyntaxHighlightingDataBuilder builder = new SyntaxHighlightingDataBuilder();
     builder.registerHighlightingRule(0, 10, "k");
     builder.registerHighlightingRule(8, 15, "k");
+    builder.build();
   }
 }
diff --git a/sonar-batch/src/test/java/org/sonar/batch/mediumtest/highlighting/HighlightingMediumTest.java b/sonar-batch/src/test/java/org/sonar/batch/mediumtest/highlighting/HighlightingMediumTest.java
index b9fab7ba450..6b0501ec9fc 100644
--- a/sonar-batch/src/test/java/org/sonar/batch/mediumtest/highlighting/HighlightingMediumTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/mediumtest/highlighting/HighlightingMediumTest.java
@@ -23,8 +23,10 @@ import com.google.common.collect.ImmutableMap;
 import org.apache.commons.io.FileUtils;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
+import org.junit.rules.TestName;
 import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.sensor.highlighting.HighlightingBuilder;
 import org.sonar.batch.mediumtest.BatchMediumTester;
@@ -38,9 +40,12 @@ import static org.fest.assertions.Assertions.assertThat;
 
 public class HighlightingMediumTest {
 
-  @org.junit.Rule
+  @Rule
   public TemporaryFolder temp = new TemporaryFolder();
 
+  @Rule
+  public TestName testName = new TestName();
+
   public BatchMediumTester tester = BatchMediumTester.builder()
     .registerPlugin("xoo", new XooPlugin())
     .addDefaultQProfile("xoo", "Sonar Way")
@@ -89,4 +94,46 @@ public class HighlightingMediumTest {
 
   }
 
+  @Test
+  public void computeSyntaxHighlightingOnBigFile() throws IOException {
+
+    File baseDir = temp.newFolder();
+    File srcDir = new File(baseDir, "src");
+    srcDir.mkdir();
+
+    File xooFile = new File(srcDir, "sample.xoo");
+    File xoohighlightingFile = new File(srcDir, "sample.xoo.highlighting");
+    FileUtils.write(xooFile, "Sample xoo\ncontent");
+    int chunkSize = 100000;
+    StringBuilder sb = new StringBuilder(16 * chunkSize);
+    for (int i = 0; i < chunkSize; i++) {
+      sb.append(i).append(":").append(i + 1).append(":s\n");
+    }
+    FileUtils.write(xoohighlightingFile, sb.toString());
+
+    long start = System.currentTimeMillis();
+    TaskResult result = tester.newTask()
+      .properties(ImmutableMap.<String, String>builder()
+        .put("sonar.task", "scan")
+        .put("sonar.projectBaseDir", baseDir.getAbsolutePath())
+        .put("sonar.projectKey", "com.foo.project")
+        .put("sonar.projectName", "Foo Project")
+        .put("sonar.projectVersion", "1.0-SNAPSHOT")
+        .put("sonar.projectDescription", "Description of Foo Project")
+        .put("sonar.sources", "src")
+        .build())
+      .start();
+    long duration = System.currentTimeMillis() - start;
+    assertDurationLessThan(duration, 20000L);
+
+    InputFile file = result.inputFiles().get(0);
+    assertThat(result.highlightingTypeFor(file, 0)).isEqualTo(HighlightingBuilder.TypeOfText.STRING);
+
+  }
+
+  void assertDurationLessThan(long duration, long maxDuration) {
+    assertThat(duration).as(String.format("Expected less than %d ms, got %d ms", maxDuration, duration)).isLessThanOrEqualTo(maxDuration);
+    System.out.printf("Test %s : %d ms (max allowed is %d)\n", testName.getMethodName(), duration, maxDuration);
+  }
+
 }
diff --git a/sonar-batch/src/test/java/org/sonar/batch/source/DefaultHighlightableTest.java b/sonar-batch/src/test/java/org/sonar/batch/source/DefaultHighlightableTest.java
index 2f98e07cc79..efa8024c6a9 100644
--- a/sonar-batch/src/test/java/org/sonar/batch/source/DefaultHighlightableTest.java
+++ b/sonar-batch/src/test/java/org/sonar/batch/source/DefaultHighlightableTest.java
@@ -44,7 +44,7 @@ public class DefaultHighlightableTest {
     DefaultHighlightable highlightablePerspective = new DefaultHighlightable(mock(Component.class), null);
     highlightablePerspective.newHighlighting().highlight(0, 10, "k").highlight(20, 30, "cppd");
 
-    assertThat(highlightablePerspective.getHighlightingRules().getSortedRules()).hasSize(2);
+    assertThat(highlightablePerspective.getHighlightingRules().getSyntaxHighlightingRuleSet()).hasSize(2);
   }
 
   @Test
diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/highlighting/HighlightingBuilder.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/highlighting/HighlightingBuilder.java
index db8d5c46e93..0acc7848e22 100644
--- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/highlighting/HighlightingBuilder.java
+++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/highlighting/HighlightingBuilder.java
@@ -19,7 +19,6 @@
  */
 package org.sonar.api.batch.sensor.highlighting;
 
-
 /**
  * This builder is used to define syntax highlighting (aka code coloration) on files.
  * @since 4.5
@@ -73,6 +72,7 @@ public interface HighlightingBuilder {
 
   /**
    * Call this method only once when your are done with defining highlighting of the file.
+   * @throws IllegalStateException if you have defined overlapping highlighting
    */
   void done();
 }