Double the speed of DecimalFormat creation.

Our calls to unum_setSymbol were making us O(n^2); switching to the C++ API
and doing a bulk update is a huge win. (ICU is really a C++ library with a
C wrapper. It's always going to be slightly wasteful to go via C, but here
it's especially harmful.)

The new ScopedJavaUnicodeString provides a best-of-breed bridge between Java
strings on the Java heap and the UnicodeString type that ICU wants. I'll come
back and switch more of our ICU JNI over in a later patch.

3 files changed, 124 insertions, 76 deletions

diff --git a/icu/src/main/java/com/ibm/icu4jni/text/NativeDecimalFormat.java b/icu/src/main/java/com/ibm/icu4jni/text/NativeDecimalFormat.java
index 034491c..7cb5de2 100644
--- a/icu/src/main/java/com/ibm/icu4jni/text/NativeDecimalFormat.java
+++ b/icu/src/main/java/com/ibm/icu4jni/text/NativeDecimalFormat.java
@@ -118,7 +118,7 @@ public class NativeDecimalFormat {
     private BigDecimal multiplierBigDecimal = null;
 
     public NativeDecimalFormat(String pattern, Locale locale, DecimalFormatSymbols symbols) {
-        this.addr = openDecimalFormat(locale.toString(), pattern);
+        this.addr = openDecimalFormat(pattern);
         this.lastPattern = pattern;
         setDecimalFormatSymbols(symbols);
     }
@@ -188,27 +188,14 @@ public class NativeDecimalFormat {
     }
 
     /**
-     * Copies the DecimalFormatSymbols settings into our native peer.
+     * Copies the DecimalFormatSymbols settings into our native peer in bulk.
      */
     public void setDecimalFormatSymbols(final DecimalFormatSymbols dfs) {
-        setSymbol(this.addr, UNUM_CURRENCY_SYMBOL, dfs.getCurrencySymbol());
-
-        setSymbol(this.addr, UNUM_DECIMAL_SEPARATOR_SYMBOL, dfs.getDecimalSeparator());
-        setSymbol(this.addr, UNUM_DIGIT_SYMBOL, dfs.getDigit());
-
-        char groupingSeparator = dfs.getGroupingSeparator();
-        setSymbol(this.addr, UNUM_GROUPING_SEPARATOR_SYMBOL, groupingSeparator);
-        setSymbol(this.addr, UNUM_MONETARY_GROUPING_SEPARATOR_SYMBOL, groupingSeparator);
-
-        setSymbol(this.addr, UNUM_INFINITY_SYMBOL, dfs.getInfinity());
-        setSymbol(this.addr, UNUM_INTL_CURRENCY_SYMBOL, dfs.getInternationalCurrencySymbol());
-        setSymbol(this.addr, UNUM_MINUS_SIGN_SYMBOL, dfs.getMinusSign());
-        setSymbol(this.addr, UNUM_MONETARY_SEPARATOR_SYMBOL, dfs.getMonetaryDecimalSeparator());
-        setSymbol(this.addr, UNUM_NAN_SYMBOL, dfs.getNaN());
-        setSymbol(this.addr, UNUM_PATTERN_SEPARATOR_SYMBOL, dfs.getPatternSeparator());
-        setSymbol(this.addr, UNUM_PERCENT_SYMBOL, dfs.getPercent());
-        setSymbol(this.addr, UNUM_PERMILL_SYMBOL, dfs.getPerMill());
-        setSymbol(this.addr, UNUM_ZERO_DIGIT_SYMBOL, dfs.getZeroDigit());
+        setDecimalFormatSymbols(this.addr, dfs.getCurrencySymbol(), dfs.getDecimalSeparator(),
+                dfs.getDigit(), dfs.getGroupingSeparator(), dfs.getInfinity(),
+                dfs.getInternationalCurrencySymbol(), dfs.getMinusSign(),
+                dfs.getMonetaryDecimalSeparator(), dfs.getNaN(), dfs.getPatternSeparator(),
+                dfs.getPercent(), dfs.getPerMill(), dfs.getZeroDigit());
     }
 
     private BigDecimal applyMultiplier(BigDecimal valBigDecimal) {
@@ -573,10 +560,10 @@ public class NativeDecimalFormat {
         return null;
     }
 
-    private static int openDecimalFormat(String locale, String pattern) {
+    private static int openDecimalFormat(String pattern) {
         try {
             // FIXME: if we're about to override everything, should we just ask for the cheapest locale (presumably the root locale)?
-            return openDecimalFormatImpl(locale, pattern);
+            return openDecimalFormatImpl(pattern);
         } catch (NullPointerException npe) {
             throw npe;
         } catch (RuntimeException re) {
@@ -604,8 +591,12 @@ public class NativeDecimalFormat {
     // FIXME: do we need getSymbol any more? the Java-side object should be the canonical source.
     private static native String getSymbol(int addr, int symbol);
     private static native String getTextAttribute(int addr, int symbol);
-    private static native int openDecimalFormatImpl(String locale, String pattern);
+    private static native int openDecimalFormatImpl(String pattern);
     private static native Number parse(int addr, String string, ParsePosition position);
+    private static native void setDecimalFormatSymbols(int addr, String currencySymbol,
+            char decimalSeparator, char digit, char groupingSeparator, String infinity,
+            String internationalCurrencySymbol, char minusSign, char monetaryDecimalSeparator,
+            String nan, char patternSeparator, char percent, char perMill, char zeroDigit);
     private static native void setSymbol(int addr, int symbol, String str);
     private static native void setSymbol(int addr, int symbol, char ch);
     private static native void setAttribute(int addr, int symbol, int i);
diff --git a/icu/src/main/native/NativeDecimalFormat.cpp b/icu/src/main/native/NativeDecimalFormat.cpp
index e974521..ba62726 100644
--- a/icu/src/main/native/NativeDecimalFormat.cpp
+++ b/icu/src/main/native/NativeDecimalFormat.cpp
@@ -25,6 +25,7 @@
 #include "unicode/ustring.h"
 #include "digitlst.h"
 #include "ErrorCode.h"
+#include "ScopedJavaUnicodeString.h"
 #include <stdlib.h>
 #include <string.h>
 
@@ -33,47 +34,33 @@ static void jniThrowNullPointerException(JNIEnv* env) {
     jniThrowException(env, "java/lang/NullPointerException", NULL);
 }
 
-DecimalFormat* toDecimalFormat(jint addr) {
+static DecimalFormat* toDecimalFormat(jint addr) {
     return reinterpret_cast<DecimalFormat*>(static_cast<uintptr_t>(addr));
 }
 
-static jint openDecimalFormatImpl(JNIEnv* env, jclass clazz, jstring locale, jstring pattern) {
-    if (pattern == NULL) {
+static jint openDecimalFormatImpl(JNIEnv* env, jclass clazz, jstring pattern0) {
+    if (pattern0 == NULL) {
         jniThrowNullPointerException(env);
         return 0;
     }
 
-    // prepare the pattern string for the call to unum_open
-    const UChar *pattChars = env->GetStringChars(pattern, NULL);
-    int pattLen = env->GetStringLength(pattern);
-
-    // prepare the locale string for the call to unum_open
-    const char *localeChars = env->GetStringUTFChars(locale, NULL);
-
-    // open a default type number format
     UErrorCode status = U_ZERO_ERROR;
-    UNumberFormat *fmt = unum_open(UNUM_PATTERN_DECIMAL, pattChars, pattLen,
-            localeChars, NULL, &status);
-
-    // release the allocated strings
-    env->ReleaseStringChars(pattern, pattChars);
-    env->ReleaseStringUTFChars(locale, localeChars);
-
+    UParseError parseError;
+    ScopedJavaUnicodeString pattern(env, pattern0);
+    DecimalFormatSymbols* symbols = new DecimalFormatSymbols(status);
+    DecimalFormat* fmt = new DecimalFormat(pattern.unicodeString(), symbols, parseError, status);
+    if (fmt == NULL) {
+        delete symbols;
+    }
     icu4jni_error(env, status);
     return static_cast<jint>(reinterpret_cast<uintptr_t>(fmt));
 }
 
 static void closeDecimalFormatImpl(JNIEnv *env, jclass clazz, jint addr) {
-
-    // get the pointer to the number format
-    UNumberFormat *fmt = (UNumberFormat *)(int)addr;
-
-    // close this number format
-    unum_close(fmt);
+    delete toDecimalFormat(addr);
 }
 
-static void setSymbol(JNIEnv *env, uintptr_t addr, jint symbol,
-        const UChar* chars, int32_t charCount) {
+static void setSymbol(JNIEnv *env, uintptr_t addr, jint symbol, const UChar* chars, int32_t charCount) {
     UErrorCode status = U_ZERO_ERROR;
     UNumberFormat* fmt = reinterpret_cast<UNumberFormat*>(static_cast<uintptr_t>(addr));
     unum_setSymbol(fmt, static_cast<UNumberFormatSymbol>(symbol),
@@ -92,6 +79,32 @@ static void setSymbol_char(JNIEnv *env, jclass, jint addr, jint symbol, jchar ch
     setSymbol(env, addr, symbol, &ch, 1);
 }
 
+static void setDecimalFormatSymbols(JNIEnv* env, jclass, jint addr, jstring currencySymbol0, jchar decimalSeparator, jchar digit, jchar groupingSeparator0, jstring infinity0, jstring internationalCurrencySymbol0, jchar minusSign, jchar monetaryDecimalSeparator, jstring nan0, jchar patternSeparator, jchar percent, jchar perMill, jchar zeroDigit) {
+    ScopedJavaUnicodeString currencySymbol(env, currencySymbol0);
+    ScopedJavaUnicodeString infinity(env, infinity0);
+    ScopedJavaUnicodeString internationalCurrencySymbol(env, internationalCurrencySymbol0);
+    ScopedJavaUnicodeString nan(env, nan0);
+    UnicodeString groupingSeparator(groupingSeparator0);
+
+    DecimalFormat* fmt = toDecimalFormat(addr);
+    DecimalFormatSymbols newSymbols(*fmt->getDecimalFormatSymbols());
+    newSymbols.setSymbol(DecimalFormatSymbols::kCurrencySymbol, currencySymbol.unicodeString());
+    newSymbols.setSymbol(DecimalFormatSymbols::kDecimalSeparatorSymbol, UnicodeString(decimalSeparator));
+    newSymbols.setSymbol(DecimalFormatSymbols::kDigitSymbol, UnicodeString(digit));
+    newSymbols.setSymbol(DecimalFormatSymbols::kGroupingSeparatorSymbol, groupingSeparator);
+    newSymbols.setSymbol(DecimalFormatSymbols::kMonetaryGroupingSeparatorSymbol, groupingSeparator);
+    newSymbols.setSymbol(DecimalFormatSymbols::kInfinitySymbol, infinity.unicodeString());
+    newSymbols.setSymbol(DecimalFormatSymbols::kIntlCurrencySymbol, internationalCurrencySymbol.unicodeString());
+    newSymbols.setSymbol(DecimalFormatSymbols::kMinusSignSymbol, UnicodeString(minusSign));
+    newSymbols.setSymbol(DecimalFormatSymbols::kMonetarySeparatorSymbol, UnicodeString(monetaryDecimalSeparator));
+    newSymbols.setSymbol(DecimalFormatSymbols::kNaNSymbol, nan.unicodeString());
+    newSymbols.setSymbol(DecimalFormatSymbols::kPatternSeparatorSymbol, UnicodeString(patternSeparator));
+    newSymbols.setSymbol(DecimalFormatSymbols::kPercentSymbol, UnicodeString(percent));
+    newSymbols.setSymbol(DecimalFormatSymbols::kPerMillSymbol, UnicodeString(perMill));
+    newSymbols.setSymbol(DecimalFormatSymbols::kZeroDigitSymbol, UnicodeString(zeroDigit));
+    fmt->setDecimalFormatSymbols(newSymbols);
+}
+
 static jstring getSymbol(JNIEnv *env, jclass clazz, jint addr, jint symbol) {
 
     uint32_t resultlength, reslenneeded;
@@ -203,17 +216,19 @@ static jstring getTextAttribute(JNIEnv *env, jclass clazz, jint addr,
     return res;
 }
 
-static void applyPatternImpl(JNIEnv *env, jclass clazz, jint addr, jboolean localized, jstring pattern) {
-    if (pattern == NULL) {
+static void applyPatternImpl(JNIEnv *env, jclass clazz, jint addr, jboolean localized, jstring pattern0) {
+    if (pattern0 == NULL) {
         jniThrowNullPointerException(env);
         return;
     }
-    UNumberFormat* fmt = reinterpret_cast<UNumberFormat*>(static_cast<uintptr_t>(addr));
-    const UChar* chars = env->GetStringChars(pattern, NULL);
-    jsize charCount = env->GetStringLength(pattern);
+    ScopedJavaUnicodeString pattern(env, pattern0);
+    DecimalFormat* fmt = toDecimalFormat(addr);
     UErrorCode status = U_ZERO_ERROR;
-    unum_applyPattern(fmt, localized, chars, charCount, NULL, &status);
-    env->ReleaseStringChars(pattern, chars);
+    if (localized) {
+        fmt->applyLocalizedPattern(pattern.unicodeString(), status);
+    } else {
+        fmt->applyPattern(pattern.unicodeString(), status);
+    }
     icu4jni_error(env, status);
 }
 
@@ -589,31 +604,23 @@ static jint cloneDecimalFormatImpl(JNIEnv *env, jclass, jint addr) {
 
 static JNINativeMethod gMethods[] = {
     /* name, signature, funcPtr */
-    {"openDecimalFormatImpl", "(Ljava/lang/String;Ljava/lang/String;)I",
-            (void*) openDecimalFormatImpl},
+    {"applyPatternImpl", "(IZLjava/lang/String;)V", (void*) applyPatternImpl},
+    {"cloneDecimalFormatImpl", "(I)I", (void*) cloneDecimalFormatImpl},
     {"closeDecimalFormatImpl", "(I)V", (void*) closeDecimalFormatImpl},
-    {"setSymbol", "(IIC)V", (void*) setSymbol_char},
-    {"setSymbol", "(IILjava/lang/String;)V", (void*) setSymbol_String},
+    {"format", "(IDLjava/text/FieldPosition;Ljava/lang/String;Ljava/lang/StringBuffer;)Ljava/lang/String;", (void*) formatDouble},
+    {"format", "(IJLjava/text/FieldPosition;Ljava/lang/String;Ljava/lang/StringBuffer;)Ljava/lang/String;", (void*) formatLong},
+    {"format", "(ILjava/lang/String;Ljava/text/FieldPosition;Ljava/lang/String;Ljava/lang/StringBuffer;I)Ljava/lang/String;", (void*) formatDigitList},
+    {"getAttribute", "(II)I", (void*) getAttribute},
     {"getSymbol", "(II)Ljava/lang/String;", (void*) getSymbol},
+    {"getTextAttribute", "(II)Ljava/lang/String;", (void*) getTextAttribute},
+    {"openDecimalFormatImpl", "(Ljava/lang/String;)I", (void*) openDecimalFormatImpl},
+    {"parse", "(ILjava/lang/String;Ljava/text/ParsePosition;)Ljava/lang/Number;", (void*) parse},
     {"setAttribute", "(III)V", (void*) setAttribute},
-    {"getAttribute", "(II)I", (void*) getAttribute},
+    {"setDecimalFormatSymbols", "(ILjava/lang/String;CCCLjava/lang/String;Ljava/lang/String;CCLjava/lang/String;CCCC)V", (void*) setDecimalFormatSymbols},
+    {"setSymbol", "(IIC)V", (void*) setSymbol_char},
+    {"setSymbol", "(IILjava/lang/String;)V", (void*) setSymbol_String},
     {"setTextAttribute", "(IILjava/lang/String;)V", (void*) setTextAttribute},
-    {"getTextAttribute", "(II)Ljava/lang/String;", (void*) getTextAttribute},
-    {"applyPatternImpl", "(IZLjava/lang/String;)V", (void*) applyPatternImpl},
     {"toPatternImpl", "(IZ)Ljava/lang/String;", (void*) toPatternImpl},
-    {"format",
-            "(IJLjava/text/FieldPosition;Ljava/lang/String;Ljava/lang/StringBuffer;)Ljava/lang/String;",
-            (void*) formatLong},
-    {"format",
-            "(IDLjava/text/FieldPosition;Ljava/lang/String;Ljava/lang/StringBuffer;)Ljava/lang/String;",
-            (void*) formatDouble},
-    {"format",
-            "(ILjava/lang/String;Ljava/text/FieldPosition;Ljava/lang/String;Ljava/lang/StringBuffer;I)Ljava/lang/String;",
-            (void*) formatDigitList},
-    {"parse",
-            "(ILjava/lang/String;Ljava/text/ParsePosition;)Ljava/lang/Number;",
-            (void*) parse},
-    {"cloneDecimalFormatImpl", "(I)I", (void*) cloneDecimalFormatImpl}
 };
 int register_com_ibm_icu4jni_text_NativeDecimalFormat(JNIEnv* env) {
     return jniRegisterNativeMethods(env,
diff --git a/icu/src/main/native/ScopedJavaUnicodeString.h b/icu/src/main/native/ScopedJavaUnicodeString.h
new file mode 100644
index 0000000..3486aac
--- /dev/null
+++ b/icu/src/main/native/ScopedJavaUnicodeString.h
@@ -0,0 +1,50 @@
+/*
+ * Copyright (C) 2010 The Android Open Source Project
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef SCOPED_JAVA_UNICODE_STRING_H_included
+#define SCOPED_JAVA_UNICODE_STRING_H_included
+
+#include "JNIHelp.h"
+
+// A smart pointer that provides access to an ICU UnicodeString given a JNI
+// jstring. We give ICU a direct pointer to the characters on the Java heap.
+// It's clever enough to copy-on-write if necessary, but we only provide
+// const UnicodeString access anyway because attempted write access seems
+// likely to be an error.
+class ScopedJavaUnicodeString {
+public:
+    ScopedJavaUnicodeString(JNIEnv* env, jstring s) : mEnv(env), mString(s) {
+        mChars = env->GetStringChars(mString, NULL);
+        const int32_t charCount = env->GetStringLength(mString);
+        mUnicodeString.setTo(false, mChars, charCount);
+    }
+
+    ~ScopedJavaUnicodeString() {
+        mEnv->ReleaseStringChars(mString, mChars);
+    }
+
+    const UnicodeString& unicodeString() {
+        return mUnicodeString;
+    }
+
+private:
+    JNIEnv* mEnv;
+    jstring mString;
+    const UChar* mChars;
+    UnicodeString mUnicodeString;
+};
+
+#endif  // SCOPED_JAVA_UNICODE_STRING_H_included