vsch / flexmark-java

CommonMark/Markdown Java parser with source level AST. CommonMark 0.28, emulation of: pegdown, kramdown, markdown.pl, MultiMarkdown. With HTML to MD, MD to PDF, MD to DOCX conversion modules.
BSD 2-Clause "Simplified" License
2.28k stars 271 forks source link

Performance issue with regex in HtmlConverterCoreNodeRenderer #633

Open praveen-diffbot opened 1 month ago

praveen-diffbot commented 1 month ago

HtmlConverterCoreNodeRenderer.handleTableCell has a call to String.replaceAll("\\s*\n\\s*", " ") which can be quite slow. The regex is quite simple and can be sped up by removing the regex.

To Reproduce

See attached file test.html.txt

public class LoadingTest {
  public static void main(final String[] args) throws Exception {
    final String STR = java.nio.file.Files.readString(java.nio.file.Path.of("test.html.txt"));
    final long tic = System.currentTimeMillis();
    com.diffbot.websearch.html.MarkdownNormalizer.markdown(STR);
    System.out.println("took: " + (System.currentTimeMillis() - tic));
  }
}

Expected behavior The code takes >4000 ms to run on my laptop.

took: 4024

It should take much lesser time.

praveen-diffbot commented 1 month ago

I replaced the regex call with an approximation which is probably ok for html

 private String replaceMultipleBlankSpace(String cellText) {
     StringBuilder result = new StringBuilder();
     boolean wasSpace = false;

     for (char c : cellText.toCharArray()) {
         if (Character.isWhitespace(c)) {
             if (!wasSpace) {
                 result.append(' ');
                 wasSpace = true;
             }
         } else {
             result.append(c);
             wasSpace = false;
         }
     }

     return result.toString();
}
String cellText = replaceMultipleBlankSpace(context.processTextNodes(element).trim());

The same file takes about 150ms to process

DarkTyger commented 3 weeks ago

Here's a micro test showing the behaviour of the regular expression versus the non-regex solution:

public class T {
  private static final String[] INPUTS = {
    "Line one\nLine two",
    "   Leading spaces\nTrailing spaces   ",
    "Multiple  spaces between words",
    "\n\nBlank lines\n\n",
    "NoSpaces"
  };

  private static final String[] OUTPUTS = {
    "Line one Line two",
    "Leading spaces Trailing spaces",
    "Multiple  spaces between words",
    "Blank lines",
    "NoSpaces"
  };

  public static void main(final String[] args) {
    for (var i = 0; i < INPUTS.length; i++) {
      final var result = regex(INPUTS[i]);

      if (!result.equals(OUTPUTS[i])) {
        System.out.println("Test failed:");
        System.out.println("Input: " + INPUTS[i]);
        System.out.println("Expected: " + OUTPUTS[i]);
        System.out.println("Got: " + result);
        return;
      }
    }

    System.out.println("ORIGINAL tests passed.");

    for (var i = 0; i < INPUTS.length; i++) {
      final var result = revision(INPUTS[i]);

      if (!result.equals(OUTPUTS[i])) {
        System.out.println("REVISION test failed!");
        System.out.println("Input: " + INPUTS[i]);
        System.out.println("Expected: " + OUTPUTS[i]);
        System.out.println("Got: " + result);
        return;
      }
    }

    System.out.println("ALL tests passed.");
  }

  private static String regex(final String text) {
    return text.trim().replaceAll("\\s*\n\\s*", " ");
  }

  private static String revision(final String text) {
    final var result = new StringBuilder(text.length());
    boolean wasSpace = false;

    for (final var c : text.toCharArray()) {
      final var isSpace = Character.isWhitespace(c);
      final var toAppend = isSpace ? ' ' : c;

      if (!wasSpace || !isSpace) {
        result.append(toAppend);
      }

      wasSpace = isSpace;
    }

    return result.toString().trim();
  }
}

I've simplified the algorithm and showed the edge case that fails.

The procedural implementation is not the same as the regular expression.

Notice that instantiating the StringBuilder with text.length() will have modest performance improvements for large strings.

Have you tried pre-compiling the regex as a Pattern constant, instead, to see if there are any performance gains?