]> granicus.if.org Git - icu/commitdiff
ICU-13634 Fixing lazy-compute call site and other minor changes.
authorShane Carr <shane@unicode.org>
Fri, 20 Apr 2018 01:32:53 +0000 (01:32 +0000)
committerShane Carr <shane@unicode.org>
Fri, 20 Apr 2018 01:32:53 +0000 (01:32 +0000)
X-SVN-Rev: 41252

icu4c/source/i18n/currunit.cpp
icu4c/source/i18n/decimfmt.cpp
icu4c/source/i18n/number_patternstring.cpp
icu4c/source/i18n/number_patternstring.h

index b65a67c112b1507008016993c0dc33583d429285..3de6856a2189f7263e3d98ff0daa0174a0e23c7c 100644 (file)
@@ -38,7 +38,7 @@ CurrencyUnit::CurrencyUnit(ConstChar16Ptr _isoCode, UErrorCode& ec) {
         isoCodeToUse = _isoCode;
     }
     // TODO: Perform uppercasing here like in ICU4J Currency.getInstance()?
-    u_strncpy(isoCode, isoCodeToUse, 3);
+    uprv_memcpy(isoCode, isoCodeToUse, sizeof(UChar) * 3);
     isoCode[3] = 0;
     char simpleIsoCode[4];
     u_UCharsToChars(isoCode, simpleIsoCode, 4);
index 0e0d81fcfcf105de30d679dadd72c80e5b1d345d..213321078133ffb27f55d4ef658d98780ebe486b 100644 (file)
@@ -383,8 +383,8 @@ DecimalFormat& DecimalFormat::operator=(const DecimalFormat& rhs) {
 }
 
 DecimalFormat::~DecimalFormat() {
-    delete fAtomicParser.load();
-    delete fAtomicCurrencyParser.load();
+    delete fAtomicParser.exchange(nullptr);
+    delete fAtomicCurrencyParser.exchange(nullptr);
 };
 
 Format* DecimalFormat::clone() const {
@@ -1078,8 +1078,8 @@ void DecimalFormat::touch(UErrorCode& status) {
                             locale)));
 
     // Delete the parsers if they were made previously
-    delete fAtomicParser.exchange(nullptr, std::memory_order_relaxed);
-    delete fAtomicCurrencyParser.exchange(nullptr, std::memory_order_relaxed);
+    delete fAtomicParser.exchange(nullptr);
+    delete fAtomicCurrencyParser.exchange(nullptr);
 
     // In order for the getters to work, we need to populate some fields in NumberFormat.
     NumberFormat::setCurrency(fExportedProperties->currency.get(status).getISOCurrency(), status);
@@ -1104,41 +1104,57 @@ void DecimalFormat::setPropertiesFromPattern(const UnicodeString& pattern, int32
 }
 
 const numparse::impl::NumberParserImpl& DecimalFormat::getParser(UErrorCode& status) const {
+    // First try to get the pre-computed parser
     auto* ptr = fAtomicParser.load();
     if (ptr != nullptr) {
         return *ptr;
     }
 
-    ptr = NumberParserImpl::createParserFromProperties(*fProperties, *fSymbols, false, status);
-
-    if (ptr == nullptr) {
+    // Try computing the parser on our own
+    auto* temp = NumberParserImpl::createParserFromProperties(*fProperties, *fSymbols, false, status);
+    if (temp == nullptr) {
         status = U_MEMORY_ALLOCATION_ERROR;
-        // although we still dereference, call sites should be guarded
+        // although we may still dereference, call sites should be guarded
     }
 
-    // Store the new pointer, and delete the old one if it got created.
+    // Note: ptr starts as nullptr; during compare_exchange, it is set to what is actually stored in the
+    // atomic, which may me temp or may be another object computed by a different thread.
     auto* nonConstThis = const_cast<DecimalFormat*>(this);
-    delete nonConstThis->fAtomicParser.exchange(ptr, std::memory_order_relaxed);
-    return *ptr;
+    if (!nonConstThis->fAtomicParser.compare_exchange_strong(ptr, temp)) {
+        // Another thread beat us to computing the parser
+        delete temp;
+        return *ptr;
+    } else {
+        // Our copy of the parser got stored in the atomic
+        return *temp;
+    }
 }
 
 const numparse::impl::NumberParserImpl& DecimalFormat::getCurrencyParser(UErrorCode& status) const {
+    // First try to get the pre-computed parser
     auto* ptr = fAtomicCurrencyParser.load();
     if (ptr != nullptr) {
         return *ptr;
     }
 
-    ptr = NumberParserImpl::createParserFromProperties(*fProperties, *fSymbols, true, status);
-
-    if (ptr == nullptr) {
+    // Try computing the parser on our own
+    auto* temp = NumberParserImpl::createParserFromProperties(*fProperties, *fSymbols, true, status);
+    if (temp == nullptr) {
         status = U_MEMORY_ALLOCATION_ERROR;
-        // although we still dereference, call sites should be guarded
+        // although we may still dereference, call sites should be guarded
     }
 
-    // Store the new pointer, and delete the old one if it got created.
+    // Note: ptr starts as nullptr; during compare_exchange, it is set to what is actually stored in the
+    // atomic, which may me temp or may be another object computed by a different thread.
     auto* nonConstThis = const_cast<DecimalFormat*>(this);
-    delete nonConstThis->fAtomicCurrencyParser.exchange(ptr, std::memory_order_relaxed);
-    return *ptr;
+    if (!nonConstThis->fAtomicCurrencyParser.compare_exchange_strong(ptr, temp)) {
+        // Another thread beat us to computing the parser
+        delete temp;
+        return *ptr;
+    } else {
+        // Our copy of the parser got stored in the atomic
+        return *temp;
+    }
 }
 
 void DecimalFormat::setupFastFormat() {
index f967fb4bba6e23eb30f60f41f1c925b5bcd02700..21bccbab697d64551d6cce25123721ba12dccf64 100644 (file)
@@ -865,8 +865,8 @@ int PatternStringUtils::escapePaddingString(UnicodeString input, UnicodeString&
 }
 
 UnicodeString
-PatternStringUtils::convertLocalized(UnicodeString input, DecimalFormatSymbols symbols, bool toLocalized,
-                                     UErrorCode& status) {
+PatternStringUtils::convertLocalized(const UnicodeString& input, const DecimalFormatSymbols& symbols,
+                                     bool toLocalized, UErrorCode& status) {
     // Construct a table of strings to be converted between localized and standard.
     static constexpr int32_t LEN = 21;
     UnicodeString table[LEN][2];
index a87cc7415c326d3c7cbba8f2c7587cc123239455..fc4100d8035b73c67531562190aa9909d504db87 100644 (file)
@@ -264,7 +264,7 @@ class U_I18N_API PatternStringUtils {
      *            notation.
      * @return The pattern expressed in the other notation.
      */
-    static UnicodeString convertLocalized(UnicodeString input, DecimalFormatSymbols symbols,
+    static UnicodeString convertLocalized(const UnicodeString& input, const DecimalFormatSymbols& symbols,
                                           bool toLocalized, UErrorCode& status);
 
     /**