From ca350d8a82fceb83d0d7166156abfc71307b2efb Mon Sep 17 00:00:00 2001 From: Shane Carr Date: Wed, 4 Oct 2017 01:15:54 +0000 Subject: [PATCH] ICU-13177 Removing public dependency on std::atomic from unicode/numberformatter.h and casting to ICU's atomic type internally. X-SVN-Rev: 40540 --- icu4c/source/i18n/number_fluent.cpp | 43 +++++++++++++++------ icu4c/source/i18n/unicode/numberformatter.h | 14 +++---- 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/icu4c/source/i18n/number_fluent.cpp b/icu4c/source/i18n/number_fluent.cpp index 00c9eb1b4ed..4f18d6e361b 100644 --- a/icu4c/source/i18n/number_fluent.cpp +++ b/icu4c/source/i18n/number_fluent.cpp @@ -7,6 +7,7 @@ #include "unicode/numberformatter.h" #include "number_decimalquantity.h" #include "number_formatimpl.h" +#include "umutex.h" using namespace icu; using namespace icu::number; @@ -107,7 +108,7 @@ Derived NumberFormatterSettings::padding(const Padder &padder) const { } template -Derived NumberFormatterSettings::threshold(uint32_t threshold) const { +Derived NumberFormatterSettings::threshold(int32_t threshold) const { Derived copy(*this); copy.fMacros.threshold = threshold; return copy; @@ -240,7 +241,7 @@ const NumberingSystem* SymbolsWrapper::getNumberingSystem() const { } LocalizedNumberFormatter::~LocalizedNumberFormatter() { - delete fCompiled.load(); + delete fCompiled; } FormattedNumber LocalizedNumberFormatter::formatInt(int64_t value, UErrorCode &status) const { @@ -278,19 +279,39 @@ FormattedNumber LocalizedNumberFormatter::formatDecimal(StringPiece value, UErro FormattedNumber LocalizedNumberFormatter::formatImpl(impl::NumberFormatterResults *results, UErrorCode &status) const { - uint32_t currentCount = fCallCount.load(); - if (currentCount <= fMacros.threshold && fMacros.threshold > 0) { - currentCount = const_cast(this)->fCallCount.fetch_add(1) + 1; + // fUnsafeCallCount contains memory to be interpreted as an atomic int, most commonly + // std::atomic. Since the type of atomic int is platform-dependent, we cast the + // bytes in fUnsafeCallCount to u_atomic_int32_t, a typedef for the platform-dependent + // atomic int type defined in umutex.h. + static_assert(sizeof(u_atomic_int32_t) <= sizeof(fUnsafeCallCount), + "Atomic integer size on this platform exceeds the size allocated by fUnsafeCallCount"); + u_atomic_int32_t* callCount = reinterpret_cast( + const_cast(this)->fUnsafeCallCount); + + // A positive value in the atomic int indicates that the data structure is not yet ready; + // a negative value indicates that it is ready. If, after the increment, the atomic int + // is exactly threshold, then it is the current thread's job to build the data structure. + // Note: We set the callCount to INT32_MIN so that if another thread proceeds to increment + // the atomic int, the value remains below zero. + int32_t currentCount = umtx_loadAcquire(*callCount); + if (0 <= currentCount && currentCount <= fMacros.threshold && fMacros.threshold > 0) { + currentCount = umtx_atomic_inc(callCount); } - const NumberFormatterImpl *compiled; + if (currentCount == fMacros.threshold && fMacros.threshold > 0) { - compiled = NumberFormatterImpl::fromMacros(fMacros, status); - U_ASSERT(fCompiled.load() == nullptr); - const_cast(this)->fCompiled.store(compiled); - compiled->apply(results->quantity, results->string, status); - } else if ((compiled = fCompiled.load()) != nullptr) { + // Build the data structure and then use it (slow to fast path). + const NumberFormatterImpl* compiled = + NumberFormatterImpl::fromMacros(fMacros, status); + U_ASSERT(fCompiled == nullptr); + const_cast(this)->fCompiled = compiled; + umtx_storeRelease(*callCount, INT32_MIN); compiled->apply(results->quantity, results->string, status); + } else if (currentCount < 0) { + // The data structure is already built; use it (fast path). + U_ASSERT(fCompiled != nullptr); + fCompiled->apply(results->quantity, results->string, status); } else { + // Format the number without building the data structure (slow path). NumberFormatterImpl::applyStatic(fMacros, results->quantity, results->string, status); } diff --git a/icu4c/source/i18n/unicode/numberformatter.h b/icu4c/source/i18n/unicode/numberformatter.h index 7afda66824a..6c02aa12b0e 100644 --- a/icu4c/source/i18n/unicode/numberformatter.h +++ b/icu4c/source/i18n/unicode/numberformatter.h @@ -5,8 +5,6 @@ #ifndef __NUMBERFORMATTER_H__ #define __NUMBERFORMATTER_H__ -#include - #include "unicode/appendable.h" #include "unicode/dcfmtsym.h" #include "unicode/currunit.h" @@ -1146,7 +1144,7 @@ namespace impl { * * @internal */ -static uint32_t DEFAULT_THRESHOLD = 3; +static constexpr int32_t DEFAULT_THRESHOLD = 3; /** @internal */ class U_I18N_API SymbolsWrapper : public UMemory { @@ -1321,7 +1319,7 @@ struct U_I18N_API MacroProps : public UMemory { PluralRules *rules = nullptr; // no ownership /** @internal */ - uint32_t threshold = DEFAULT_THRESHOLD; + int32_t threshold = DEFAULT_THRESHOLD; Locale locale; /** @@ -1701,7 +1699,7 @@ class U_I18N_API NumberFormatterSettings { * * @internal ICU 60: This API is ICU internal only. */ - Derived threshold(uint32_t threshold) const; + Derived threshold(int32_t threshold) const; #endif /* U_HIDE_INTERNAL_API */ @@ -1818,10 +1816,8 @@ class U_I18N_API LocalizedNumberFormatter ~LocalizedNumberFormatter(); private: - UPRV_SUPPRESS_DLL_INTERFACE_WARNING // Member is private and does not need to be exported - std::atomic fCompiled{nullptr}; - UPRV_SUPPRESS_DLL_INTERFACE_WARNING // Member is private and does not need to be exported - std::atomic fCallCount{0}; + const impl::NumberFormatterImpl* fCompiled {nullptr}; + char fUnsafeCallCount[8] {}; // internally cast to u_atomic_int32_t LocalizedNumberFormatter() = default;