From 07a3890c024848faca5254ac47ecd573c7e6d8f3 Mon Sep 17 00:00:00 2001 From: Travis Keep Date: Wed, 30 Apr 2014 21:26:08 +0000 Subject: [PATCH] ICU-10793 Optimize ListFormatter. X-SVN-Rev: 35677 --- .../ibm/icu/impl/SimplePatternFormatter.java | 47 ++++++++++++++----- .../src/com/ibm/icu/text/ListFormatter.java | 40 ++++++++-------- .../test/util/SimplePatternFormatterTest.java | 29 ++++++++++-- 3 files changed, 81 insertions(+), 35 deletions(-) diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/SimplePatternFormatter.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/SimplePatternFormatter.java index 9609b933e3f..f64943788b5 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/SimplePatternFormatter.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/SimplePatternFormatter.java @@ -121,6 +121,16 @@ public class SimplePatternFormatter { return placeholderCount; } + /** + * Returns true if this instance starts with placeholder with given id. + */ + public boolean startsWithPlaceholder(int id) { + if (placeholderIdsOrderedByOffset.length == 0) { + return false; + } + return (placeholderIdsOrderedByOffset[0] == 0 && placeholderIdsOrderedByOffset[1] == id); + } + /** * Formats the given values. */ @@ -131,8 +141,12 @@ public class SimplePatternFormatter { /** * Formats the given values. * - * @param appendTo the result appended here. - * @param offsets position of first value in appendTo stored in offfsets[0]; + * @param appendTo the result appended here. Optimization: If the pattern this object + * represents starts with a placeholder AND appendTo references the value of that same + * placeholder (corresponding values parameter must also be a StringBuilder), then that + * placeholder value is not copied to appendTo (Its already there). If the value of the + * starting placeholder is very large, this optimization can offer huge savings. + * @param offsets position of first value in appendTo stored in offsets[0]; * second in offsets[1]; third in offsets[2] etc. An offset of -1 means that the * corresponding value is not in appendTo. offsets.length and values.length may * differ. If caller is not interested in offsets, caller may pass null here. @@ -152,16 +166,25 @@ public class SimplePatternFormatter { appendTo.append(patternWithoutPlaceholders); return appendTo; } - appendTo.append( - patternWithoutPlaceholders, - 0, - placeholderIdsOrderedByOffset[0]); - setPlaceholderOffset( - placeholderIdsOrderedByOffset[1], - appendTo.length(), - offsets, - offsetLen); - appendTo.append(values[placeholderIdsOrderedByOffset[1]]); + if (placeholderIdsOrderedByOffset[0] > 0 || + appendTo != values[placeholderIdsOrderedByOffset[1]]) { + appendTo.append( + patternWithoutPlaceholders, + 0, + placeholderIdsOrderedByOffset[0]); + setPlaceholderOffset( + placeholderIdsOrderedByOffset[1], + appendTo.length(), + offsets, + offsetLen); + appendTo.append(values[placeholderIdsOrderedByOffset[1]]); + } else { + setPlaceholderOffset( + placeholderIdsOrderedByOffset[1], + 0, + offsets, + offsetLen); + } for (int i = 2; i < placeholderIdsOrderedByOffset.length; i += 2) { appendTo.append( patternWithoutPlaceholders, diff --git a/icu4j/main/classes/core/src/com/ibm/icu/text/ListFormatter.java b/icu4j/main/classes/core/src/com/ibm/icu/text/ListFormatter.java index c570752c3e4..b892325537d 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/text/ListFormatter.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/text/ListFormatter.java @@ -256,13 +256,13 @@ final public class ListFormatter { // Builds a formatted list static class FormattedListBuilder { - private String current; + private StringBuilder current; private int offset; // Start is the first object in the list; If recordOffset is true, records the offset of // this first object. public FormattedListBuilder(Object start, boolean recordOffset) { - this.current = start.toString(); + this.current = new StringBuilder(start.toString()); this.offset = recordOffset ? 0 : -1; } @@ -274,28 +274,28 @@ final public class ListFormatter { if (pattern.getPlaceholderCount() != 2) { throw new IllegalArgumentException("Need {0} and {1} only in pattern " + pattern); } - if (recordOffset || offsetRecorded()) { - int[] offsets = new int[2]; - current = pattern.format( - new StringBuilder(), offsets, current, next.toString()).toString(); - if (offsets[0] == -1 || offsets[1] == -1) { - throw new IllegalArgumentException( - "{0} or {1} missing from pattern " + pattern); - } - if (recordOffset) { - offset = offsets[1]; - } else { - offset += offsets[0]; - } - } else { - current = pattern.format(current, next.toString()); - } - return this; + int[] offsets = (recordOffset || offsetRecorded()) ? new int[2] : null; + StringBuilder nextBuilder = + pattern.startsWithPlaceholder(0) ? current : new StringBuilder(); + current = pattern.format( + nextBuilder, offsets, current, next.toString()); + if (offsets != null) { + if (offsets[0] == -1 || offsets[1] == -1) { + throw new IllegalArgumentException( + "{0} or {1} missing from pattern " + pattern); + } + if (recordOffset) { + offset = offsets[1]; + } else { + offset += offsets[0]; + } + } + return this; } @Override public String toString() { - return current; + return current.toString(); } // Gets the last recorded offset or -1 if no offset recorded. diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/util/SimplePatternFormatterTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/util/SimplePatternFormatterTest.java index 2bf6031ab73..3ff2aef7e2f 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/util/SimplePatternFormatterTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/util/SimplePatternFormatterTest.java @@ -37,7 +37,7 @@ public class SimplePatternFormatterTest extends TestFmwk { 0, fmt.getPlaceholderCount()); assertEquals( - "evaluate", + "format", "This doesn't have templates {0}", fmt.format("unused")); assertEquals( @@ -59,7 +59,7 @@ public class SimplePatternFormatterTest extends TestFmwk { 0, fmt.getPlaceholderCount()); assertEquals( - "evaluate", + "format", "Some {} messed {12d up stuff.", fmt.format("to")); } @@ -73,6 +73,7 @@ public class SimplePatternFormatterTest extends TestFmwk { public void TestWithPlaceholders() { SimplePatternFormatter fmt = SimplePatternFormatter.compile( "Templates {2}{1} and {4} are out of order."); + assertFalse("startsWithPlaceholder", fmt.startsWithPlaceholder(2)); assertEquals( "getPlaceholderCount", 5, @@ -89,7 +90,7 @@ public class SimplePatternFormatterTest extends TestFmwk { fmt.toString()); int[] offsets = new int[6]; assertEquals( - "evaluate", + "format", "123456: Templates frogtommy and {0} are out of order.", fmt.format( new StringBuilder("123456: "), @@ -103,4 +104,26 @@ public class SimplePatternFormatterTest extends TestFmwk { } } } + + public void TestOptimization() { + SimplePatternFormatter fmt = SimplePatternFormatter.compile("{2}, {0}, {1} and {3}"); + assertTrue("startsWithPlaceholder", fmt.startsWithPlaceholder(2)); + assertFalse("startsWithPlaceholder", fmt.startsWithPlaceholder(0)); + int[] offsets = new int[4]; + StringBuilder appendTo = new StringBuilder("leg"); + assertEquals( + "format", + "leg, freddy, frog and by", + fmt.format( + appendTo, + offsets, + "freddy", "frog", appendTo, "by").toString()); + + int[] expectedOffsets = {5, 13, 0, 22}; + for (int i = 0; i < offsets.length; i++) { + if (offsets[i] != expectedOffsets[i]) { + fail("getOffset() returned wrong value for " + i); + } + } + } } -- 2.40.0