Bug 56595: Also switch the cache in DateUtil.isADateFormat() to ThreadLocals to not have another syncpoint here. Again only very little data is kept, so no memory bloat should happen because of this.

Also do more simple checks before actually looking at the cache

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1647296 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Dominik Stadler 2014-12-22 12:08:59 +00:00
parent 43ee4e08ca
commit a7ba500280
2 changed files with 66 additions and 33 deletions

View File

@ -327,14 +327,29 @@ public class DateUtil {
return calendar;
}
// variables for performance optimization:
// avoid re-checking DataUtil.isADateFormat(int, String) if a given format
// string represents a date format if the same string is passed multiple times.
// see https://issues.apache.org/bugzilla/show_bug.cgi?id=55611
private static int lastFormatIndex = -1;
private static String lastFormatString = null;
private static boolean cached = false;
private static ThreadLocal<Integer> lastFormatIndex = new ThreadLocal<Integer>() {
protected Integer initialValue() {
return -1;
}
};
private static ThreadLocal<String> lastFormatString = new ThreadLocal<String>();
private static ThreadLocal<Boolean> lastCachedResult = new ThreadLocal<Boolean>();
private static boolean isCached(String formatString, int formatIndex) {
String cachedFormatString = lastFormatString.get();
return cachedFormatString != null && formatIndex == lastFormatIndex.get()
&& formatString.equals(cachedFormatString);
}
private static void cache(String formatString, int formatIndex, boolean cached) {
lastFormatIndex.set(formatIndex);
lastFormatString.set(formatString);
lastCachedResult.set(Boolean.valueOf(cached));
}
/**
* Given a format ID and its format String, will check to see if the
@ -350,27 +365,23 @@ public class DateUtil {
* @see #isInternalDateFormat(int)
*/
public static synchronized boolean isADateFormat(int formatIndex, String formatString) {
if (formatString != null && formatIndex == lastFormatIndex && formatString.equals(lastFormatString)) {
return cached;
}
public static boolean isADateFormat(int formatIndex, String formatString) {
// First up, is this an internal date format?
if(isInternalDateFormat(formatIndex)) {
lastFormatIndex = formatIndex;
lastFormatString = formatString;
cached = true;
cache(formatString, formatIndex, true);
return true;
}
// If we didn't get a real string, it can't be
// If we didn't get a real string, don't even cache it as we can always find this out quickly
if(formatString == null || formatString.length() == 0) {
lastFormatIndex = formatIndex;
lastFormatString = formatString;
cached = false;
return false;
}
// check the cache first
if (isCached(formatString, formatIndex)) {
return lastCachedResult.get();
}
String fs = formatString;
/*if (false) {
// Normalize the format string. The code below is equivalent
@ -419,9 +430,7 @@ public class DateUtil {
// short-circuit if it indicates elapsed time: [h], [m] or [s]
if(date_ptrn4.matcher(fs).matches()){
lastFormatIndex = formatIndex;
lastFormatString = formatString;
cached = true;
cache(formatString, formatIndex, true);
return true;
}
@ -449,9 +458,7 @@ public class DateUtil {
// optionally followed by AM/PM
boolean result = date_ptrn3b.matcher(fs).matches();
lastFormatIndex = formatIndex;
lastFormatString = formatString;
cached = result;
cache(formatString, formatIndex, result);
return result;
}

View File

@ -21,6 +21,7 @@
package org.apache.poi.ss.usermodel;
import java.io.IOException;
import java.text.DateFormat;
import java.util.Calendar;
import java.util.Date;
@ -158,7 +159,7 @@ public class TestDataFormatter extends TestCase {
String p2dp_n1dp_z0 = "00.00;(00.0);0";
String all2dpTSP = "00.00_x";
String p2dp_n2dpTSP = "00.00_x;(00.00)_x";
String p2dp_n1dpTSP = "00.00_x;(00.0)_x";
//String p2dp_n1dpTSP = "00.00_x;(00.0)_x";
assertEquals("12.34", dfUS.formatRawCellContents(12.343, -1, all2dp));
assertEquals("12.34", dfUS.formatRawCellContents(12.343, -1, p2dp_n1dp));
@ -493,20 +494,24 @@ public class TestDataFormatter extends TestCase {
assertEquals(" $- ", dfUS.formatRawCellContents(0.0, -1, "_-$* #,##0.00_-;-$* #,##0.00_-;_-$* \"-\"??_-;_-@_-"));
}
public void testErrors() {
public void testErrors() throws IOException {
DataFormatter dfUS = new DataFormatter(Locale.US, true);
// Create a spreadsheet with some formula errors in it
Workbook wb = new HSSFWorkbook();
Sheet s = wb.createSheet();
Row r = s.createRow(0);
Cell c = r.createCell(0, Cell.CELL_TYPE_ERROR);
try {
Sheet s = wb.createSheet();
Row r = s.createRow(0);
Cell c = r.createCell(0, Cell.CELL_TYPE_ERROR);
c.setCellErrorValue(FormulaError.DIV0.getCode());
assertEquals(FormulaError.DIV0.getString(), dfUS.formatCellValue(c));
c.setCellErrorValue(FormulaError.DIV0.getCode());
assertEquals(FormulaError.DIV0.getString(), dfUS.formatCellValue(c));
c.setCellErrorValue(FormulaError.REF.getCode());
assertEquals(FormulaError.REF.getString(), dfUS.formatCellValue(c));
c.setCellErrorValue(FormulaError.REF.getCode());
assertEquals(FormulaError.REF.getString(), dfUS.formatCellValue(c));
} finally {
wb.close();
}
}
/**
@ -607,4 +612,25 @@ public class TestDataFormatter extends TestCase {
assertTrue(e.getMessage().contains("Cannot format given Object as a Number"));
}
}
public void testIsADateFormat() {
// first check some cases that should not be a date, also call multiple times to ensure the cache is used
assertFalse(DateUtil.isADateFormat(-1, null));
assertFalse(DateUtil.isADateFormat(-1, null));
assertFalse(DateUtil.isADateFormat(123, null));
assertFalse(DateUtil.isADateFormat(123, ""));
assertFalse(DateUtil.isADateFormat(124, ""));
assertFalse(DateUtil.isADateFormat(-1, ""));
assertFalse(DateUtil.isADateFormat(-1, ""));
assertFalse(DateUtil.isADateFormat(-1, "nodateformat"));
// then also do the same for some valid date formats
assertTrue(DateUtil.isADateFormat(0x0e, null));
assertTrue(DateUtil.isADateFormat(0x2f, null));
assertTrue(DateUtil.isADateFormat(-1, "yyyy"));
assertTrue(DateUtil.isADateFormat(-1, "yyyy"));
assertTrue(DateUtil.isADateFormat(-1, "dd/mm/yy;[red]dd/mm/yy"));
assertTrue(DateUtil.isADateFormat(-1, "dd/mm/yy;[red]dd/mm/yy"));
assertTrue(DateUtil.isADateFormat(-1, "[h]"));
}
}