ASF Bugzilla – Attachment 29588 Details for
Bug 54137
PATCH Patch for performance issues with DataFormatter Fractions
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
Initail patch file to limit impact
patch.txt (text/plain), 9.89 KB, created by
Alan Davis
on 2012-11-12 11:34:00 UTC
(
hide
)
Description:
Initail patch file to limit impact
Filename:
MIME Type:
Creator:
Alan Davis
Created:
2012-11-12 11:34:00 UTC
Size:
9.89 KB
patch
obsolete
>Index: src/java/org/apache/poi/ss/usermodel/DataFormatter.java >=================================================================== >--- src/java/org/apache/poi/ss/usermodel/DataFormatter.java (revision 1407532) >+++ src/java/org/apache/poi/ss/usermodel/DataFormatter.java (working copy) >@@ -13,6 +13,10 @@ > WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > See the License for the specific language governing permissions and > limitations under the License. >+ >+ 2012 - Alfresco Software, Ltd. >+ Alfresco Software has modified source of this file >+ The details of changes as svn diff can be found in svn at location root/projects/3rd-party/src > ==================================================================== */ > package org.apache.poi.ss.usermodel; > >@@ -248,6 +252,12 @@ > } > > private Format getFormat(double cellValue, int formatIndex, String formatStrIn) { >+// // Might be better to separate out the n p and z formats, falling back to p when n and z are not set. >+// // That however would require other code to be re factored. >+// String[] formatBits = formatStrIn.split(";"); >+// int i = cellValue > 0.0 ? 0 : cellValue < 0.0 ? 1 : 2; >+// String formatStr = (i < formatBits.length) ? formatBits[i] : formatBits[0]; >+ > String formatStr = formatStrIn; > // Excel supports positive/negative/zero, but java > // doesn't, so we need to do it specially >@@ -364,10 +374,21 @@ > } > > // Excel supports fractions in format strings, which Java doesn't >- if (!formatStr.contains("-") && >- (formatStr.indexOf("#/#") >= 0 && formatStr.indexOf("#/#") == formatStr.lastIndexOf("#/#")) || >- (formatStr.indexOf("?/?") >= 0 && formatStr.indexOf("?/?") == formatStr.lastIndexOf("?/?"))) { >- return new FractionFormat(formatStr); >+ if (formatStr.indexOf("#/#") >= 0 || formatStr.indexOf("?/?") >= 0) { >+ // Strip custom text in quotes and escaped characters for now as it can cause performance problems in fractions. >+ String strippedFormatStr = formatStr.replaceAll("\\\\ ", " ").replaceAll("\\\\.", "").replaceAll("\"[^\"]*\"", " "); >+ >+ boolean ok = true; >+ for (String part: strippedFormatStr.split(";")) { >+ int indexOfFraction = indexOfFraction(part); >+ if (indexOfFraction == -1 || indexOfFraction != lastIndexOfFraction(part)) { >+ ok = false; >+ break; >+ } >+ } >+ if (ok) { >+ return new FractionFormat(strippedFormatStr); >+ } > } > > if (numPattern.matcher(formatStr).find()) { >@@ -380,7 +401,19 @@ > // TODO - when does this occur? > return null; > } >+ >+ private int indexOfFraction(String format) { >+ int i = format.indexOf("#/#"); >+ int j = format.indexOf("?/?"); >+ return i == -1 ? j : j == -1 ? i : Math.min(i, j); >+ } > >+ private int lastIndexOfFraction(String format) { >+ int i = format.lastIndexOf("#/#"); >+ int j = format.lastIndexOf("?/?"); >+ return i == -1 ? j : j == -1 ? i : Math.max(i, j); >+ } >+ > private Format createDateFormat(String pFormatStr, double cellValue) { > String formatStr = pFormatStr; > formatStr = formatStr.replaceAll("\\\\-","-"); >@@ -996,14 +1029,26 @@ > } > > public String format(Number num) { >- double wholePart = Math.floor(num.doubleValue()); >- double decPart = num.doubleValue() - wholePart; >+ >+ double doubleValue = num.doubleValue(); >+ >+ // Format may be p or p;n or p;n;z (okay we never get a z). >+ // Fall back to p when n or z is not specified. >+ String[] formatBits = str.split(";"); >+ int f = doubleValue > 0.0 ? 0 : doubleValue < 0.0 ? 1 : 2; >+ String str = (f < formatBits.length) ? formatBits[f] : formatBits[0]; >+ >+ double wholePart = Math.floor(Math.abs(doubleValue)); >+ double decPart = Math.abs(doubleValue) - wholePart; > if (wholePart + decPart == 0) { > return "0"; > } >- >+ if (doubleValue < 0.0) { >+ wholePart *= -1.0; >+ } >+ > // Split the format string into decimal and fraction parts >- String[] parts = str.split(" "); >+ String[] parts = str.replaceAll(" *", " ").split(" "); > String[] fractParts; > if (parts.length == 2) { > fractParts = parts[1].split("/"); >@@ -1017,11 +1062,12 @@ > } > > if (fractParts.length == 2) { >+ int fractPart1Length = Math.min(countHashes(fractParts[1]), 4); // Any more than 3 and we go around the loops for ever > double minVal = 1.0; >- double currDenom = Math.pow(10 , fractParts[1].length()) - 1d; >+ double currDenom = Math.pow(10 , fractPart1Length) - 1d; > double currNeum = 0; >- for (int i = (int)(Math.pow(10, fractParts[1].length())- 1d); i > 0; i--) { >- for(int i2 = (int)(Math.pow(10, fractParts[1].length())- 1d); i2 > 0; i2--){ >+ for (int i = (int)(Math.pow(10, fractPart1Length)- 1d); i > 0; i--) { >+ for(int i2 = (int)(Math.pow(10, fractPart1Length)- 1d); i2 > 0; i2--){ > if (minVal >= Math.abs((double)i2/(double)i - decPart)) { > currDenom = i; > currNeum = i2; >@@ -1040,9 +1086,19 @@ > return result; > } > } else { >- throw new IllegalArgumentException("Fraction must have 2 parts, found " + fractParts.length + " for fraction format " + str); >+ throw new IllegalArgumentException("Fraction must have 2 parts, found " + fractParts.length + " for fraction format " + this.str); > } > } >+ >+ private int countHashes(String format) { >+ int count = 0; >+ for (int i=format.length()-1; i >= 0; i--) { >+ if (format.charAt(i) == '#') { >+ count++; >+ } >+ } >+ return count; >+ } > > public StringBuffer format(Object obj, StringBuffer toAppendTo, FieldPosition pos) { > return toAppendTo.append(format((Number)obj)); >Index: src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java >=================================================================== >--- src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java (revision 1407532) >+++ src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java (working copy) >@@ -13,6 +13,10 @@ > WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > See the License for the specific language governing permissions and > limitations under the License. >+ >+ 2012 - Alfresco Software, Ltd. >+ Alfresco Software has modified source of this file >+ The details of changes as svn diff can be found in svn at location root/projects/3rd-party/src > ==================================================================== */ > > package org.apache.poi.ss.usermodel; >@@ -195,6 +199,41 @@ > assertEquals("321 1/3", dfUS.formatRawCellContents(321.321, -1, "# ?/?")); > assertEquals("321 26/81", dfUS.formatRawCellContents(321.321, -1, "# ?/??")); > assertEquals("26027/81", dfUS.formatRawCellContents(321.321, -1, "?/??")); >+ >+ // p;n;z;s parts >+ assertEquals( "321 1/3", dfUS.formatRawCellContents(321.321, -1, "# #/#;# ##/#;0;xxx")); >+ assertEquals("-321 1/3", dfUS.formatRawCellContents(-321.321, -1, "# #/#;# ##/#;0;xxx")); >+ assertEquals("0", dfUS.formatRawCellContents(0, -1, "# #/#;# ##/#;0;xxx")); >+// assertEquals("0.0", dfUS.formatRawCellContents(0, -1, "# #/#;# ##/#;#.#;xxx")); // currently hard coded to 0 >+ >+ // Custom formats with text are not currently supported >+// assertEquals("+ve", dfUS.formatRawCellContents(0, -1, "+ve;-ve;zero;xxx")); >+// assertEquals("-ve", dfUS.formatRawCellContents(0, -1, "-ve;-ve;zero;xxx")); >+// assertEquals("zero", dfUS.formatRawCellContents(0, -1, "zero;-ve;zero;xxx")); >+ >+ // Custom formats - check text is stripped, including multiple spaces >+ assertEquals("321 1/3", dfUS.formatRawCellContents(321.321, -1, "# #/#")); >+ assertEquals("321 1/3", dfUS.formatRawCellContents(321.321, -1, "#\" \" #/#")); >+ assertEquals("321 1/3", dfUS.formatRawCellContents(321.321, -1, "#\"FRED\" #/#")); >+ assertEquals("321 1/3", dfUS.formatRawCellContents(321.321, -1, "#\\ #/#")); >+ assertEquals("321 1/3", dfUS.formatRawCellContents(321.321, -1, "# \\q#/#")); >+ >+ // Cases that were very slow >+ assertEquals("321 1/3", dfUS.formatRawCellContents(321.321, -1, "0\" \"?/?;?/?")); // 0" "?/?;?/? - length of -ve part was used >+ assertEquals("321 1/3", dfUS.formatRawCellContents(321.321, -1, "0 \"#\"\\#\\#?/?")); // 0 "#"\#\#?/? - length of text was used >+ >+ assertEquals("321 295/919", dfUS.formatRawCellContents(321.321, -1, "# #/###")); >+ assertEquals("321 321/1000", dfUS.formatRawCellContents(321.321, -1, "# #/####")); // Code limits to #### as that is as slow as we want to get >+ assertEquals("321 321/1000", dfUS.formatRawCellContents(321.321, -1, "# #/##########")); >+ >+ // Not a valid fraction formats (too many #/# or ?/?) - hence the strange expected results >+ assertEquals("321 / ?/?", dfUS.formatRawCellContents(321.321, -1, "# #/# ?/?")); >+ assertEquals("321 / /", dfUS.formatRawCellContents(321.321, -1, "# #/# #/#")); >+ assertEquals("321 ?/? ?/?", dfUS.formatRawCellContents(321.321, -1, "# ?/? ?/?")); >+ assertEquals("321 ?/? / /", dfUS.formatRawCellContents(321.321, -1, "# ?/? #/# #/#")); >+ >+ // Where both p and n don't include a fraction, so cannot always be formatted >+ assertEquals("123", dfUS.formatRawCellContents(-123.321, -1, "0 ?/?;0")); > } > > /**
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Actions:
View
Attachments on
bug 54137
: 29588 |
29591