ICU-21684 Fix performance issue in NumberRangeFormatter

unicode-org · Sep 15, 2021 · 13fb584 · 13fb584
1 parent aa17f26
commit 13fb584
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 2 deletions.
diff --git a/icu4c/source/i18n/numrange_impl.cpp b/icu4c/source/i18n/numrange_impl.cpp
@@ -385,7 +385,10 @@ void NumberRangeFormatterImpl::formatRange(UFormattedNumberRangeData& data,
     }
 
     length1 += NumberFormatterImpl::writeNumber(micros1, data.quantity1, string, UPRV_INDEX_0, status);
-    length2 += NumberFormatterImpl::writeNumber(micros2, data.quantity2, string, UPRV_INDEX_2, status);
+    // ICU-21684: Write the second number to a temp string to avoid repeated insert operations
+    FormattedStringBuilder tempString;
+    NumberFormatterImpl::writeNumber(micros2, data.quantity2, tempString, 0, status);
+    length2 += string.insert(UPRV_INDEX_2, tempString, status);
 
     // TODO: Support padding?
 

diff --git a/icu4c/source/test/intltest/numbertest.h b/icu4c/source/test/intltest/numbertest.h
@@ -317,6 +317,7 @@ class NumberRangeFormatterTest : public IntlTestWithFieldPosition {
     void testCopyMove();
     void toObject();
     void testGetDecimalNumbers();
+    void test21684_Performance();
     void test21358_SignPosition();
 
     void runIndexedTest(int32_t index, UBool exec, const char *&name, char *par = 0) override;

diff --git a/icu4c/source/test/intltest/numbertest_range.cpp b/icu4c/source/test/intltest/numbertest_range.cpp
@@ -54,6 +54,7 @@ void NumberRangeFormatterTest::runIndexedTest(int32_t index, UBool exec, const c
         TESTCASE_AUTO(testCopyMove);
         TESTCASE_AUTO(toObject);
         TESTCASE_AUTO(testGetDecimalNumbers);
+        TESTCASE_AUTO(test21684_Performance);
         TESTCASE_AUTO(test21358_SignPosition);
     TESTCASE_AUTO_END;
 }
@@ -930,6 +931,14 @@ void NumberRangeFormatterTest::testGetDecimalNumbers() {
     }
 }
 
+void NumberRangeFormatterTest::test21684_Performance() {
+    IcuTestErrorCode status(*this, "test21684_Performance");
+    LocalizedNumberRangeFormatter lnf = NumberRangeFormatter::withLocale("en");
+    // The following two lines of code should finish quickly.
+    lnf.formatFormattableRange({"-1e99999", status}, {"0", status}, status);
+    lnf.formatFormattableRange({"0", status}, {"1e99999", status}, status);
+}
+
 void NumberRangeFormatterTest::test21358_SignPosition() {
     IcuTestErrorCode status(*this, "test21358_SignPosition");
 

diff --git a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberRangeFormatterImpl.java b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberRangeFormatterImpl.java
@@ -364,7 +364,10 @@ private void formatRange(DecimalQuantity quantity1, DecimalQuantity quantity2, F
         }
 
         h.length1 += NumberFormatterImpl.writeNumber(micros1, quantity1, string, h.index0());
-        h.length2 += NumberFormatterImpl.writeNumber(micros2, quantity2, string, h.index2());
+        // ICU-21684: Write the second number to a temp string to avoid repeated insert operations
+        FormattedStringBuilder tempString = new FormattedStringBuilder();
+        NumberFormatterImpl.writeNumber(micros2, quantity2, tempString, 0);
+        h.length2 += string.insert(h.index2(), tempString);
 
         // TODO: Support padding?
 

diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberRangeFormatterTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberRangeFormatterTest.java
@@ -2,6 +2,7 @@
 // License & terms of use: http://www.unicode.org/copyright.html
 package com.ibm.icu.dev.test.number;
 
+import java.math.BigDecimal;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -872,6 +873,14 @@ public void testNumberingSystemRangeData() {
         }
     }
 
+    @Test
+    public void test21684_Performance() {
+        LocalizedNumberRangeFormatter lnf = NumberRangeFormatter.withLocale(ULocale.ENGLISH);
+        // The following two lines of code should finish quickly.
+        lnf.formatRange(new BigDecimal("-1e99999"), new BigDecimal("0"));
+        lnf.formatRange(new BigDecimal("0"), new BigDecimal("1e99999"));
+    }
+
     @Test
     public void test21358_SignPosition() {
         // de-CH has currency pattern "¤ #,##0.00;¤-#,##0.00"