ICU-13634 Refactoring getPrefixSuffix methods. In ICU4C, the pattern modifier is now accessed directly. In ICU4J, they use the same detour through the pipeline code path as before with a TODO to improve to be closer to ICU4C. In addition, in both ICU4C and ICU4J, getPrefixSuffix now uses the compiled formatter when available.

X-SVN-Rev: 41258
This commit is contained in:
Shane Carr 2018-04-21 08:01:19 +00:00
parent f412770e9d
commit e59eb48314
12 changed files with 158 additions and 71 deletions

View File

@ -614,7 +614,7 @@ void DecimalFormat::setCurrencyPluralInfo(const CurrencyPluralInfo& info) {
UnicodeString& DecimalFormat::getPositivePrefix(UnicodeString& result) const {
ErrorCode localStatus;
fFormatter->getAffix(true, false, result, localStatus);
fFormatter->getAffixImpl(true, false, result, localStatus);
return result;
}
@ -626,7 +626,7 @@ void DecimalFormat::setPositivePrefix(const UnicodeString& newValue) {
UnicodeString& DecimalFormat::getNegativePrefix(UnicodeString& result) const {
ErrorCode localStatus;
fFormatter->getAffix(true, true, result, localStatus);
fFormatter->getAffixImpl(true, true, result, localStatus);
return result;
}
@ -638,7 +638,7 @@ void DecimalFormat::setNegativePrefix(const UnicodeString& newValue) {
UnicodeString& DecimalFormat::getPositiveSuffix(UnicodeString& result) const {
ErrorCode localStatus;
fFormatter->getAffix(false, false, result, localStatus);
fFormatter->getAffixImpl(false, false, result, localStatus);
return result;
}
@ -650,7 +650,7 @@ void DecimalFormat::setPositiveSuffix(const UnicodeString& newValue) {
UnicodeString& DecimalFormat::getNegativeSuffix(UnicodeString& result) const {
ErrorCode localStatus;
fFormatter->getAffix(false, true, result, localStatus);
fFormatter->getAffixImpl(false, true, result, localStatus);
return result;
}

View File

@ -640,6 +640,34 @@ LocalizedNumberFormatter::formatDecimalQuantity(const DecimalQuantity& dq, UErro
}
void LocalizedNumberFormatter::formatImpl(impl::UFormattedNumberData* results, UErrorCode& status) const {
if (computeCompiled(status)) {
fCompiled->apply(results->quantity, results->string, status);
} else {
NumberFormatterImpl::applyStatic(fMacros, results->quantity, results->string, status);
}
}
void LocalizedNumberFormatter::getAffixImpl(bool isPrefix, bool isNegative, UnicodeString& result,
UErrorCode& status) const {
NumberStringBuilder string;
auto signum = static_cast<int8_t>(isNegative ? -1 : 1);
// Always return affixes for plural form OTHER.
static const StandardPlural::Form plural = StandardPlural::OTHER;
int32_t prefixLength;
if (computeCompiled(status)) {
prefixLength = fCompiled->getPrefixSuffix(signum, plural, string, status);
} else {
prefixLength = NumberFormatterImpl::getPrefixSuffixStatic(fMacros, signum, plural, string, status);
}
result.remove();
if (isPrefix) {
result.append(string.toTempUnicodeString().tempSubStringBetween(0, prefixLength));
} else {
result.append(string.toTempUnicodeString().tempSubStringBetween(prefixLength, string.length()));
}
}
bool LocalizedNumberFormatter::computeCompiled(UErrorCode& status) const {
// fUnsafeCallCount contains memory to be interpreted as an atomic int, most commonly
// std::atomic<int32_t>. 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
@ -666,32 +694,14 @@ void LocalizedNumberFormatter::formatImpl(impl::UFormattedNumberData* results, U
U_ASSERT(fCompiled == nullptr);
const_cast<LocalizedNumberFormatter*>(this)->fCompiled = compiled;
umtx_storeRelease(*callCount, INT32_MIN);
compiled->apply(results->quantity, results->string, status);
return true;
} 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);
return true;
} else {
// Format the number without building the data structure (slow path).
NumberFormatterImpl::applyStatic(fMacros, results->quantity, results->string, status);
}
}
void LocalizedNumberFormatter::getAffix(bool isPrefix, bool isNegative, UnicodeString& result,
UErrorCode& status) const {
NumberStringBuilder nsb;
DecimalQuantity dq;
if (isNegative) {
dq.setToInt(-1);
} else {
dq.setToInt(1);
}
int prefixLength = NumberFormatterImpl::getPrefixSuffix(fMacros, dq, nsb, status);
result.remove();
if (isPrefix) {
result.append(nsb.toTempUnicodeString().tempSubStringBetween(0, prefixLength));
} else {
result.append(nsb.toTempUnicodeString().tempSubStringBetween(prefixLength, nsb.length()));
return false;
}
}

View File

@ -73,10 +73,11 @@ void NumberFormatterImpl::applyStatic(const MacroProps& macros, DecimalQuantity&
impl.applyUnsafe(inValue, outString, status);
}
int32_t NumberFormatterImpl::getPrefixSuffix(const MacroProps& macros, DecimalQuantity& inValue,
NumberStringBuilder& outString, UErrorCode& status) {
int32_t NumberFormatterImpl::getPrefixSuffixStatic(const MacroProps& macros, int8_t signum,
StandardPlural::Form plural,
NumberStringBuilder& outString, UErrorCode& status) {
NumberFormatterImpl impl(macros, false, status);
return impl.getPrefixSuffixUnsafe(inValue, outString, status);
return impl.getPrefixSuffixUnsafe(signum, plural, outString, status);
}
// NOTE: C++ SPECIFIC DIFFERENCE FROM JAVA:
@ -101,16 +102,26 @@ void NumberFormatterImpl::applyUnsafe(DecimalQuantity& inValue, NumberStringBuil
microsToString(fMicros, inValue, outString, status);
}
int32_t
NumberFormatterImpl::getPrefixSuffixUnsafe(DecimalQuantity& inValue, NumberStringBuilder& outString,
UErrorCode& status) {
int32_t NumberFormatterImpl::getPrefixSuffix(int8_t signum, StandardPlural::Form plural,
NumberStringBuilder& outString, UErrorCode& status) const {
if (U_FAILURE(status)) { return 0; }
fMicroPropsGenerator->processQuantity(inValue, fMicros, status);
// #13453: DecimalFormat wants the affixes from the pattern only (modMiddle, aka pattern modifier).
// Safe path: use fImmutablePatternModifier.
const Modifier* modifier = fImmutablePatternModifier->getModifier(signum, plural);
modifier->apply(outString, 0, 0, status);
if (U_FAILURE(status)) { return 0; }
// #13453: DecimalFormat wants the affixes from the pattern only (modMiddle).
fMicros.modMiddle->apply(outString, 0, 0, status);
return modifier->getPrefixLength(status);
}
int32_t NumberFormatterImpl::getPrefixSuffixUnsafe(int8_t signum, StandardPlural::Form plural,
NumberStringBuilder& outString, UErrorCode& status) {
if (U_FAILURE(status)) { return 0; }
return fMicros.modMiddle->getPrefixLength(status);
// #13453: DecimalFormat wants the affixes from the pattern only (modMiddle, aka pattern modifier).
// Unsafe path: use fPatternModifier.
fPatternModifier->setNumberProperties(signum, plural);
fPatternModifier->apply(outString, 0, 0, status);
if (U_FAILURE(status)) { return 0; }
return fPatternModifier->getPrefixLength(status);
}
NumberFormatterImpl::NumberFormatterImpl(const MacroProps& macros, bool safe, UErrorCode& status) {

View File

@ -43,13 +43,20 @@ class NumberFormatterImpl : public UMemory {
* @return The index into the output at which the prefix ends and the suffix starts; in other words,
* the prefix length.
*/
static int32_t getPrefixSuffix(const MacroProps& macros, DecimalQuantity& inValue,
NumberStringBuilder& outString, UErrorCode& status);
static int32_t getPrefixSuffixStatic(const MacroProps& macros, int8_t signum,
StandardPlural::Form plural, NumberStringBuilder& outString,
UErrorCode& status);
/**
* Evaluates the "safe" MicroPropsGenerator created by "fromMacros".
*/
void apply(DecimalQuantity &inValue, NumberStringBuilder &outString, UErrorCode &status) const;
void apply(DecimalQuantity& inValue, NumberStringBuilder& outString, UErrorCode& status) const;
/**
* Like getPrefixSuffixStatic() but uses the safe compiled object.
*/
int32_t getPrefixSuffix(int8_t signum, StandardPlural::Form plural, NumberStringBuilder& outString,
UErrorCode& status) const;
private:
// Head of the MicroPropsGenerator linked list:
@ -64,7 +71,7 @@ class NumberFormatterImpl : public UMemory {
LocalPointer<const PluralRules> fRules;
LocalPointer<const ParsedPatternInfo> fPatternInfo;
LocalPointer<const ScientificHandler> fScientificHandler;
LocalPointer<const MutablePatternModifier> fPatternModifier;
LocalPointer<MutablePatternModifier> fPatternModifier;
LocalPointer<const ImmutablePatternModifier> fImmutablePatternModifier;
LocalPointer<const LongNameHandler> fLongNameHandler;
LocalPointer<const CompactHandler> fCompactHandler;
@ -79,8 +86,8 @@ class NumberFormatterImpl : public UMemory {
void applyUnsafe(DecimalQuantity &inValue, NumberStringBuilder &outString, UErrorCode &status);
int32_t getPrefixSuffixUnsafe(DecimalQuantity& inValue, NumberStringBuilder& outString,
UErrorCode& status);
int32_t getPrefixSuffixUnsafe(int8_t signum, StandardPlural::Form plural,
NumberStringBuilder& outString, UErrorCode& status);
/**
* If rulesPtr is non-null, return it. Otherwise, return a PluralRules owned by this object for the

View File

@ -137,6 +137,15 @@ void ImmutablePatternModifier::applyToMicros(MicroProps& micros, DecimalQuantity
}
}
const Modifier* ImmutablePatternModifier::getModifier(int8_t signum, StandardPlural::Form plural) const {
if (rules == nullptr) {
return pm->getModifier(signum);
} else {
return pm->getModifier(signum, plural);
}
}
/** Used by the unsafe code path. */
MicroPropsGenerator& MutablePatternModifier::addToChain(const MicroPropsGenerator* parent) {
this->parent = parent;

View File

@ -42,6 +42,8 @@ class U_I18N_API ImmutablePatternModifier : public MicroPropsGenerator, public U
void applyToMicros(MicroProps& micros, DecimalQuantity& quantity) const;
const Modifier* getModifier(int8_t signum, StandardPlural::Form plural) const;
private:
ImmutablePatternModifier(ParameterizedModifier* pm, const PluralRules* rules,
const MicroPropsGenerator* parent);

View File

@ -2206,7 +2206,7 @@ class U_I18N_API LocalizedNumberFormatter
/** Internal method for DecimalFormat compatibility.
* @internal
*/
void getAffix(bool isPrefix, bool isNegative, UnicodeString& result, UErrorCode& status) const;
void getAffixImpl(bool isPrefix, bool isNegative, UnicodeString& result, UErrorCode& status) const;
/**
* Internal method for testing.
@ -2293,6 +2293,11 @@ class U_I18N_API LocalizedNumberFormatter
LocalizedNumberFormatter(impl::MacroProps &&macros, const Locale &locale);
/**
* @return true if the compiled formatter is available.
*/
bool computeCompiled(UErrorCode& status) const;
// To give the fluent setters access to this class's constructor:
friend class NumberFormatterSettings<UnlocalizedNumberFormatter>;
friend class NumberFormatterSettings<LocalizedNumberFormatter>;

View File

@ -244,6 +244,17 @@ public class MutablePatternModifier implements Modifier, SymbolProvider, MicroPr
micros.modMiddle = pm.getModifier(quantity.signum(), plural);
}
}
// NOTE: This method is not used in ICU4J right now.
// In ICU4C, it is used by getPrefixSuffix().
// Un-comment this method when getPrefixSuffix() is cleaned up in ICU4J.
// public Modifier getModifier(byte signum, StandardPlural plural) {
// if (rules == null) {
// return pm.getModifier(signum);
// } else {
// return pm.getModifier(signum, plural);
// }
// }
}
/** Used by the unsafe code path. */

View File

@ -5,6 +5,7 @@ package com.ibm.icu.number;
import java.math.BigInteger;
import java.util.concurrent.atomic.AtomicLongFieldUpdater;
import com.ibm.icu.impl.StandardPlural;
import com.ibm.icu.impl.Utility;
import com.ibm.icu.impl.number.DecimalQuantity;
import com.ibm.icu.impl.number.DecimalQuantity_DualStorageBCD;
@ -130,19 +131,11 @@ public class LocalizedNumberFormatter extends NumberFormatterSettings<LocalizedN
*/
@Deprecated
public FormattedNumber format(DecimalQuantity fq) {
MacroProps macros = resolve();
// NOTE: In Java, the atomic increment logic is slightly different than ICU4C.
// It seems to be more efficient to make just one function call instead of two.
// Further benchmarking is required.
long currentCount = callCount.incrementAndGet(this);
NumberStringBuilder string = new NumberStringBuilder();
if (currentCount == macros.threshold.longValue()) {
compiled = NumberFormatterImpl.fromMacros(macros);
compiled.apply(fq, string);
} else if (compiled != null) {
if (computeCompiled()) {
compiled.apply(fq, string);
} else {
NumberFormatterImpl.applyStatic(macros, fq, string);
NumberFormatterImpl.applyStatic(resolve(), fq, string);
}
return new FormattedNumber(string, fq);
}
@ -153,15 +146,37 @@ public class LocalizedNumberFormatter extends NumberFormatterSettings<LocalizedN
* {@link FormattedNumber#getFieldIterator} for similar functionality.
*/
@Deprecated
public String getAffix(boolean isPrefix, boolean isNegative) {
MacroProps macros = resolve();
NumberStringBuilder nsb = new NumberStringBuilder();
DecimalQuantity dq = new DecimalQuantity_DualStorageBCD(isNegative ? -1 : 1);
int prefixLength = NumberFormatterImpl.getPrefixSuffix(macros, dq, nsb);
if (isPrefix) {
return nsb.subSequence(0, prefixLength).toString();
public String getAffixImpl(boolean isPrefix, boolean isNegative) {
NumberStringBuilder string = new NumberStringBuilder();
byte signum = (byte) (isNegative ? -1 : 1);
// Always return affixes for plural form OTHER.
StandardPlural plural = StandardPlural.OTHER;
int prefixLength;
if (computeCompiled()) {
prefixLength = compiled.getPrefixSuffix(signum, plural, string);
} else {
return nsb.subSequence(prefixLength, nsb.length()).toString();
prefixLength = NumberFormatterImpl.getPrefixSuffixStatic(resolve(), signum, plural, string);
}
if (isPrefix) {
return string.subSequence(0, prefixLength).toString();
} else {
return string.subSequence(prefixLength, string.length()).toString();
}
}
private boolean computeCompiled() {
MacroProps macros = resolve();
// NOTE: In Java, the atomic increment logic is slightly different than ICU4C.
// It seems to be more efficient to make just one function call instead of two.
// Further benchmarking is required.
long currentCount = callCount.incrementAndGet(this);
if (currentCount == macros.threshold.longValue()) {
compiled = NumberFormatterImpl.fromMacros(macros);
return true;
} else if (compiled != null) {
return true;
} else {
return false;
}
}

View File

@ -4,9 +4,11 @@ package com.ibm.icu.number;
import com.ibm.icu.impl.CurrencyData;
import com.ibm.icu.impl.CurrencyData.CurrencyFormatInfo;
import com.ibm.icu.impl.StandardPlural;
import com.ibm.icu.impl.number.CompactData.CompactType;
import com.ibm.icu.impl.number.ConstantAffixModifier;
import com.ibm.icu.impl.number.DecimalQuantity;
import com.ibm.icu.impl.number.DecimalQuantity_DualStorageBCD;
import com.ibm.icu.impl.number.Grouper;
import com.ibm.icu.impl.number.LongNameHandler;
import com.ibm.icu.impl.number.MacroProps;
@ -63,15 +65,13 @@ class NumberFormatterImpl {
* @return The index into the output at which the prefix ends and the suffix starts; in other words,
* the prefix length.
*/
public static int getPrefixSuffix(
public static int getPrefixSuffixStatic(
MacroProps macros,
DecimalQuantity inValue,
byte signum,
StandardPlural plural,
NumberStringBuilder output) {
MicroPropsGenerator microPropsGenerator = macrosToMicroGenerator(macros, false);
MicroProps micros = microPropsGenerator.processQuantity(inValue);
// #13453: DecimalFormat wants the affixes from the pattern only (modMiddle).
micros.modMiddle.apply(output, 0, 0);
return micros.modMiddle.getPrefixLength();
return getPrefixSuffixImpl(microPropsGenerator, signum, output);
}
private static final Currency DEFAULT_CURRENCY = Currency.getInstance("XXX");
@ -87,6 +87,23 @@ class NumberFormatterImpl {
microsToString(micros, inValue, outString);
}
public int getPrefixSuffix(byte signum, StandardPlural plural, NumberStringBuilder output) {
return getPrefixSuffixImpl(microPropsGenerator, signum, output);
}
private static int getPrefixSuffixImpl(MicroPropsGenerator generator, byte signum, NumberStringBuilder output) {
// #13453: DecimalFormat wants the affixes from the pattern only (modMiddle).
// TODO: Clean this up, closer to C++. The pattern modifier is not as accessible as in C++.
// Right now, ignore the plural form, run the pipeline with number 0, and get the modifier from the result.
DecimalQuantity_DualStorageBCD quantity = new DecimalQuantity_DualStorageBCD(0);
if (signum < 0) {
quantity.negate();
}
MicroProps micros = generator.processQuantity(quantity);
micros.modMiddle.apply(output, 0, 0);
return micros.modMiddle.getPrefixLength();
}
//////////
private static boolean unitIsCurrency(MeasureUnit unit) {

View File

@ -916,7 +916,7 @@ public class DecimalFormat extends NumberFormat {
* @stable ICU 2.0
*/
public synchronized String getPositivePrefix() {
return formatter.getAffix(true, false);
return formatter.getAffixImpl(true, false);
}
/**
@ -953,7 +953,7 @@ public class DecimalFormat extends NumberFormat {
* @stable ICU 2.0
*/
public synchronized String getNegativePrefix() {
return formatter.getAffix(true, true);
return formatter.getAffixImpl(true, true);
}
/**
@ -990,7 +990,7 @@ public class DecimalFormat extends NumberFormat {
* @stable ICU 2.0
*/
public synchronized String getPositiveSuffix() {
return formatter.getAffix(false, false);
return formatter.getAffixImpl(false, false);
}
/**
@ -1027,7 +1027,7 @@ public class DecimalFormat extends NumberFormat {
* @stable ICU 2.0
*/
public synchronized String getNegativeSuffix() {
return formatter.getAffix(false, true);
return formatter.getAffixImpl(false, true);
}
/**

View File

@ -5011,7 +5011,7 @@ public class NumberFormatTest extends TestFmwk {
DecimalFormat df = (DecimalFormat) NumberFormat.getInstance();
df.applyPattern("¤¤¤ 0");
String result = df.getPositivePrefix();
assertEquals("Triple-currency should give long name on getPositivePrefix", "US dollar ", result);
assertEquals("Triple-currency should give long name on getPositivePrefix", "US dollars ", result);
}
@Test