From a7ba5002803e542cf63328fbb457d3820de3e73d Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Mon, 22 Dec 2014 12:08:59 +0000 Subject: [PATCH] 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 --- .../org/apache/poi/ss/usermodel/DateUtil.java | 51 +++++++++++-------- .../poi/ss/usermodel/TestDataFormatter.java | 48 +++++++++++++---- 2 files changed, 66 insertions(+), 33 deletions(-) diff --git a/src/java/org/apache/poi/ss/usermodel/DateUtil.java b/src/java/org/apache/poi/ss/usermodel/DateUtil.java index 2bc9f0171..79512ef10 100644 --- a/src/java/org/apache/poi/ss/usermodel/DateUtil.java +++ b/src/java/org/apache/poi/ss/usermodel/DateUtil.java @@ -327,15 +327,30 @@ 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 lastFormatIndex = new ThreadLocal() { + protected Integer initialValue() { + return -1; + } + }; + private static ThreadLocal lastFormatString = new ThreadLocal(); + private static ThreadLocal lastCachedResult = new ThreadLocal(); + 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 * format represents a date format or not. @@ -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; } diff --git a/src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java b/src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java index 06ad051a3..08f21a590 100644 --- a/src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java +++ b/src/testcases/org/apache/poi/ss/usermodel/TestDataFormatter.java @@ -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); - - c.setCellErrorValue(FormulaError.DIV0.getCode()); - assertEquals(FormulaError.DIV0.getString(), dfUS.formatCellValue(c)); - - c.setCellErrorValue(FormulaError.REF.getCode()); - assertEquals(FormulaError.REF.getString(), dfUS.formatCellValue(c)); + 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.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]")); + } }