From: Shane Carr Date: Fri, 20 Apr 2018 01:32:53 +0000 (+0000) Subject: ICU-13634 Fixing lazy-compute call site and other minor changes. X-Git-Tag: release-62-rc~200^2~13 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=f0aadfe714199c3ae28e1a451c661eba08f2ad04;p=icu ICU-13634 Fixing lazy-compute call site and other minor changes. X-SVN-Rev: 41252 --- diff --git a/icu4c/source/i18n/currunit.cpp b/icu4c/source/i18n/currunit.cpp index b65a67c112b..3de6856a218 100644 --- a/icu4c/source/i18n/currunit.cpp +++ b/icu4c/source/i18n/currunit.cpp @@ -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); diff --git a/icu4c/source/i18n/decimfmt.cpp b/icu4c/source/i18n/decimfmt.cpp index 0e0d81fcfcf..21332107813 100644 --- a/icu4c/source/i18n/decimfmt.cpp +++ b/icu4c/source/i18n/decimfmt.cpp @@ -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(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(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() { diff --git a/icu4c/source/i18n/number_patternstring.cpp b/icu4c/source/i18n/number_patternstring.cpp index f967fb4bba6..21bccbab697 100644 --- a/icu4c/source/i18n/number_patternstring.cpp +++ b/icu4c/source/i18n/number_patternstring.cpp @@ -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]; diff --git a/icu4c/source/i18n/number_patternstring.h b/icu4c/source/i18n/number_patternstring.h index a87cc7415c3..fc4100d8035 100644 --- a/icu4c/source/i18n/number_patternstring.h +++ b/icu4c/source/i18n/number_patternstring.h @@ -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); /**