From a2e084a0954ef3bb442be5ad23ca8a6eafe46557 Mon Sep 17 00:00:00 2001
From: "Steven R. Loomis" <srl295@gmail.com>
Date: Thu, 22 Feb 2024 13:36:07 -0600
Subject: [PATCH] CLDR-17371 perf improvement for DTD inclusion

- no statistically significant increase in parsing cost over 10,000 iterations.
---
 ...tory.java => DoctypeXmlStreamWrapper.java} | 97 ++++++++++++++-----
 .../org/unicode/cldr/util/XMLFileReader.java  |  2 +-
 .../org/unicode/cldr/unittest/TestBasic.java  |  4 +-
 ....java => TestDoctypeXmlStreamWrapper.java} | 23 +++--
 4 files changed, 91 insertions(+), 35 deletions(-)
 rename tools/cldr-code/src/main/java/org/unicode/cldr/util/{DTDInsertingReaderFactory.java => DoctypeXmlStreamWrapper.java} (58%)
 rename tools/cldr-code/src/test/java/org/unicode/cldr/util/{TestDTDInsertingReaderFactory.java => TestDoctypeXmlStreamWrapper.java} (83%)

diff --git a/tools/cldr-code/src/main/java/org/unicode/cldr/util/DTDInsertingReaderFactory.java b/tools/cldr-code/src/main/java/org/unicode/cldr/util/DoctypeXmlStreamWrapper.java
similarity index 58%
rename from tools/cldr-code/src/main/java/org/unicode/cldr/util/DTDInsertingReaderFactory.java
rename to tools/cldr-code/src/main/java/org/unicode/cldr/util/DoctypeXmlStreamWrapper.java
index 9f7e274d0ad..f1e9f7370e9 100644
--- a/tools/cldr-code/src/main/java/org/unicode/cldr/util/DTDInsertingReaderFactory.java
+++ b/tools/cldr-code/src/main/java/org/unicode/cldr/util/DoctypeXmlStreamWrapper.java
@@ -6,19 +6,27 @@
 import java.io.PushbackReader;
 import java.io.Reader;
 import java.io.UnsupportedEncodingException;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
+import org.unicode.cldr.icu.LDMLConstants;
 import org.xml.sax.InputSource;
 
-public class DTDInsertingReaderFactory {
+public class DoctypeXmlStreamWrapper {
+    private static final String DOCTYPE = "<!DOCTYPE";
+    private static final char DOCTYPE_CHARS[] = DOCTYPE.toCharArray();
+    private static final byte DOCTYPE_BYTES[] = DOCTYPE.getBytes(StandardCharsets.UTF_8);
+    // the string to look for:  xmlns="
+    private static final String XMLNS_EQUALS = LDMLConstants.XMLNS + "=\"";
     /**
      * Size of the input buffer, needs to be able to handle any expansion when the header is updated
      */
     public static int BUFFER_MAX_SIZE = 1024;
     /** Size of the first read, needs to contain xmlns="..." and be less than BUFFER_MAX_SIZE */
     public static int BUFFER_READ_SIZE = 512;
+
     /**
-     * Wrap a Reader in something that will automatically insert a DTD reference in place of an
-     * xmlns directive.
+     * Wrap an InputSource in something that will automatically insert a DTD reference in place of
+     * an xmlns directive.
      *
      * @throws IOException
      */
@@ -36,6 +44,7 @@ public static InputSource wrap(InputSource src) throws IOException {
         return src;
     }
 
+    /** wrap a byte oriented stream */
     public static InputStream wrap(InputStream src, String encoding) throws IOException {
         if (encoding == null) {
             encoding = "UTF-8";
@@ -49,6 +58,21 @@ public static InputStream wrap(InputStream src, String encoding) throws IOExcept
         return pr;
     }
 
+    /** wrap a char oriented stream */
+    public static Reader wrap(Reader src) throws IOException {
+        PushbackReader pr = new PushbackReader(src, BUFFER_MAX_SIZE);
+        char inbuf[] = new char[BUFFER_READ_SIZE];
+        int readlen = pr.read(inbuf);
+        if (!hasDocType(inbuf, readlen)) {
+            char buf2[] = Arrays.copyOf(inbuf, readlen);
+            inbuf = fixup(new String(buf2)).toCharArray();
+            readlen = inbuf.length;
+        }
+        pr.unread(inbuf, 0, readlen);
+        return pr;
+    }
+
+    /** Fix an input byte array, including the DOCTYPE */
     private static String fixup(byte[] inbuf, String encoding) {
         try {
             final String s = new String(inbuf, encoding);
@@ -58,10 +82,11 @@ private static String fixup(byte[] inbuf, String encoding) {
         }
     }
 
+    /** Fix an input String, including DOCTYPE */
     private static String fixup(final String s) {
         // exit if nothing matches
         for (final DtdType d : DtdType.values()) {
-            if (s.contains("xmlns=\"" + d.getNsUrl())) {
+            if (s.contains(XMLNS_EQUALS + d.getNsUrl())) {
                 return fixup(s, d);
             }
         }
@@ -69,47 +94,69 @@ private static String fixup(final String s) {
         return s;
     }
 
+    /** Fix an input String given a specific DtdType. */
     private static String fixup(String s, DtdType d) {
         int n = s.indexOf("?>");
         if (n == -1) {
             throw new IllegalArgumentException("Invalid XML prefix: ?> not found.");
         }
+        n += 2; // move the cut-point to the end of the "?>" sequence
 
         final String doctype = "\n" + d.getDoctype() + "\n";
-        final String s2 = s.substring(0, n + 2) + doctype + s.substring(n + 2);
-
-        if (false) System.out.println(s2); // DEBUG: print out updated header
+        final String s2 = s.substring(0, n) + doctype + s.substring(n);
         return s2;
     }
 
-    public static Reader wrap(Reader src) throws IOException {
-        PushbackReader pr = new PushbackReader(src, BUFFER_MAX_SIZE);
-        char inbuf[] = new char[BUFFER_READ_SIZE];
-        int readlen = pr.read(inbuf);
-        if (!hasDocType(inbuf, readlen)) {
-            char buf2[] = Arrays.copyOf(inbuf, readlen);
-            inbuf = fixup(new String(buf2)).toCharArray();
-            readlen = inbuf.length;
-        }
-        pr.unread(inbuf, 0, readlen);
-        return pr;
-    }
-
-    private static boolean hasDocType(byte[] inbuf, String encoding) {
+    private static final boolean hasDocType(byte[] inbuf, String encoding) {
         if (inbuf == null || inbuf.length == 0) return false;
+
+        // Try as utf-8/ASCII bytes - this will be the common case
+        if (arrayContains(inbuf, inbuf.length, DOCTYPE_BYTES)) return true;
+
+        // break out here
+        if (encoding == null || encoding.equals("UTF-8")) return false;
+
+        // Try 2, with encoding
         try {
             final String s = new String(inbuf, encoding);
-            return hasDocType(s.toCharArray(), s.length());
+            return s.contains(DOCTYPE);
         } catch (UnsupportedEncodingException e) {
             throw new RuntimeException("While parsing " + encoding, e);
         }
     }
 
-    private static boolean hasDocType(char[] inbuf, int readlen) {
+    private static final boolean hasDocType(char[] inbuf, int readlen) {
         if (inbuf == null || readlen <= 0) {
             return false;
         }
-        final String s = new String(inbuf, 0, readlen);
-        return (s.contains("<!DOCTYPE"));
+        return arrayContains(inbuf, readlen, DOCTYPE_CHARS);
+    }
+
+    private static boolean arrayContains(char[] inbuf, int inlen, char[] testbuf) {
+        final int testlen = testbuf.length;
+        int t = 0;
+        for (int i = 0; i < inlen; i++) {
+            if (inbuf[i] == testbuf[t]) {
+                t++;
+                if (t == testlen) return true;
+            } else {
+                t = 0;
+            }
+        }
+        return false;
+    }
+
+    private static boolean arrayContains(byte[] inbuf, int inlen, byte[] testbuf) {
+        final int testlen = testbuf.length;
+        int t = 0;
+        for (int i = 0; i < inlen; i++) {
+            if (inbuf[i] == testbuf[t]) {
+                t++;
+                if (t == testlen) return true;
+            } else {
+                t = 0;
+            }
+        }
+        return false;
     }
 }
diff --git a/tools/cldr-code/src/main/java/org/unicode/cldr/util/XMLFileReader.java b/tools/cldr-code/src/main/java/org/unicode/cldr/util/XMLFileReader.java
index 8ac2b6e0510..7c4cefe9967 100644
--- a/tools/cldr-code/src/main/java/org/unicode/cldr/util/XMLFileReader.java
+++ b/tools/cldr-code/src/main/java/org/unicode/cldr/util/XMLFileReader.java
@@ -174,7 +174,7 @@ public static void read(
         try {
             XMLReader xmlReader = createXMLReader(handlers, validating, allHandler);
             // wrap the reader to insert a character stream
-            DTDInsertingReaderFactory.wrap(is);
+            DoctypeXmlStreamWrapper.wrap(is);
             is.setSystemId(systemID);
             try {
                 xmlReader.parse(is);
diff --git a/tools/cldr-code/src/test/java/org/unicode/cldr/unittest/TestBasic.java b/tools/cldr-code/src/test/java/org/unicode/cldr/unittest/TestBasic.java
index 100ee7979a2..ebd7783451c 100644
--- a/tools/cldr-code/src/test/java/org/unicode/cldr/unittest/TestBasic.java
+++ b/tools/cldr-code/src/test/java/org/unicode/cldr/unittest/TestBasic.java
@@ -56,9 +56,9 @@
 import org.unicode.cldr.util.CharacterFallbacks;
 import org.unicode.cldr.util.CldrUtility;
 import org.unicode.cldr.util.Counter;
-import org.unicode.cldr.util.DTDInsertingReaderFactory;
 import org.unicode.cldr.util.DiscreteComparator;
 import org.unicode.cldr.util.DiscreteComparator.Ordering;
+import org.unicode.cldr.util.DoctypeXmlStreamWrapper;
 import org.unicode.cldr.util.DtdData;
 import org.unicode.cldr.util.DtdData.Attribute;
 import org.unicode.cldr.util.DtdData.Element;
@@ -377,7 +377,7 @@ public TimingInfo check(File systemID) {
             xmlReader.setErrorHandler(new MyErrorHandler());
             InputSource is = new InputSource(fis);
             is.setSystemId(systemID.toString());
-            DTDInsertingReaderFactory.wrap(is);
+            DoctypeXmlStreamWrapper.wrap(is);
             xmlReader.parse(is);
             // fis.close();
         } catch (SAXException | IOException e) {
diff --git a/tools/cldr-code/src/test/java/org/unicode/cldr/util/TestDTDInsertingReaderFactory.java b/tools/cldr-code/src/test/java/org/unicode/cldr/util/TestDoctypeXmlStreamWrapper.java
similarity index 83%
rename from tools/cldr-code/src/test/java/org/unicode/cldr/util/TestDTDInsertingReaderFactory.java
rename to tools/cldr-code/src/test/java/org/unicode/cldr/util/TestDoctypeXmlStreamWrapper.java
index f906cfa9dbc..913fa93c0c2 100644
--- a/tools/cldr-code/src/test/java/org/unicode/cldr/util/TestDTDInsertingReaderFactory.java
+++ b/tools/cldr-code/src/test/java/org/unicode/cldr/util/TestDoctypeXmlStreamWrapper.java
@@ -5,6 +5,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
@@ -13,11 +14,19 @@
 import org.xml.sax.InputSource;
 import org.xml.sax.SAXException;
 
-public class TestDTDInsertingReaderFactory {
+public class TestDoctypeXmlStreamWrapper {
     private static final int COUNT = 10; // increase this for perf testing
     private static final String COMMON_MT = "common/main/mt.xml";
     private static final String KEYBOARDS_MT = "keyboards/3.0/mt.xml";
 
+    // make sure we get some basic loading first before starting the clock
+    @BeforeAll
+    public static final void SetupStuff() throws IOException {
+        TestDoctypeXmlStreamWrapper t = new TestDoctypeXmlStreamWrapper();
+        t.TestReadJar();
+        t.TestReadCommon();
+    }
+
     @Test
     void TestProcessPathValues() {
         for (int i = 0; i < COUNT; i++) {
@@ -58,7 +67,7 @@ void TestReadKeyboardByte() throws IOException, SAXException {
         try (InputStream fis = new FileInputStream(KEYBOARDS_MT); ) {
             InputSource is = new InputSource(fis);
             is.setSystemId(KEYBOARDS_MT);
-            is = DTDInsertingReaderFactory.wrap(is);
+            is = DoctypeXmlStreamWrapper.wrap(is);
             XMLFileReader.createXMLReader(-1, true, new LoggingHandler()).parse(is);
         }
     }
@@ -70,27 +79,27 @@ void TestReadKeyboardChar() throws IOException, SAXException {
                 InputStreamReader isr = new InputStreamReader(fis); ) {
             InputSource is = new InputSource(isr);
             is.setSystemId(KEYBOARDS_MT);
-            is = DTDInsertingReaderFactory.wrap(is);
+            is = DoctypeXmlStreamWrapper.wrap(is);
             XMLFileReader.createXMLReader(-1, true, new LoggingHandler()).parse(is);
         }
     }
 
     @ParameterizedTest
-    @ValueSource(booleans = {false, true})
+    @ValueSource(booleans = {false, true, false, true})
     public void TestBytePerf(boolean wrapped) throws IOException, SAXException {
         for (int i = 0; i < COUNT; i++) {
             // mimic XMLFileHandler.read() here, but with wrapping enabled/disabled
             try (InputStream fis = new FileInputStream(COMMON_MT); ) {
                 InputSource is = new InputSource(fis);
                 is.setSystemId(COMMON_MT);
-                if (wrapped) is = DTDInsertingReaderFactory.wrap(is);
+                if (wrapped) is = DoctypeXmlStreamWrapper.wrap(is);
                 XMLFileReader.createXMLReader(-1, true, new LoggingHandler()).parse(is);
             }
         }
     }
 
     @ParameterizedTest
-    @ValueSource(booleans = {false, true})
+    @ValueSource(booleans = {false, true, false, true})
     public void TestCharPerf(boolean wrapped) throws IOException, SAXException {
         for (int i = 0; i < COUNT; i++) {
             // mimic XMLFileHandler.read() here, but with wrapping enabled/disabled
@@ -98,7 +107,7 @@ public void TestCharPerf(boolean wrapped) throws IOException, SAXException {
                     InputStreamReader isr = new InputStreamReader(fis); ) {
                 InputSource is = new InputSource(isr);
                 is.setSystemId(COMMON_MT);
-                if (wrapped) is = DTDInsertingReaderFactory.wrap(is);
+                if (wrapped) is = DoctypeXmlStreamWrapper.wrap(is);
                 XMLFileReader.createXMLReader(-1, true, new LoggingHandler()).parse(is);
             }
         }