You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@freemarker.apache.org by dd...@apache.org on 2022/08/23 16:41:42 UTC

[freemarker] 03/04: Reworked factory-level cache in JavaTemplateDateFormatFactory. Earlier, of the cache was overflown, it was simply cleared. Now, the entries flushed can be still recalled until the cache is overflown for a second time.

This is an automated email from the ASF dual-hosted git repository.

ddekany pushed a commit to branch FREEMARKER-35
in repository https://gitbox.apache.org/repos/asf/freemarker.git

commit 9584ee8461be17c14c21f503094395f775777842
Author: ddekany <dd...@apache.org>
AuthorDate: Tue Aug 23 17:29:25 2022 +0200

    Reworked factory-level cache in JavaTemplateDateFormatFactory. Earlier, of the cache was overflown, it was simply cleared. Now, the entries flushed can be still recalled until the cache is overflown for a second time.
---
 .../core/JavaTemplateDateFormatFactory.java        | 139 ++++++++++++---------
 src/manual/en_US/book.xml                          |   7 ++
 2 files changed, 86 insertions(+), 60 deletions(-)

diff --git a/src/main/java/freemarker/core/JavaTemplateDateFormatFactory.java b/src/main/java/freemarker/core/JavaTemplateDateFormatFactory.java
index 257895a1..62863c2e 100644
--- a/src/main/java/freemarker/core/JavaTemplateDateFormatFactory.java
+++ b/src/main/java/freemarker/core/JavaTemplateDateFormatFactory.java
@@ -35,10 +35,11 @@ class JavaTemplateDateFormatFactory extends TemplateDateFormatFactory {
     
     private static final Logger LOG = Logger.getLogger("freemarker.runtime");
 
-    private static final ConcurrentHashMap<CacheKey, DateFormat> GLOBAL_FORMAT_CACHE
-            = new ConcurrentHashMap<>();
-    private static final int LEAK_ALERT_DATE_FORMAT_CACHE_SIZE = 1024;
-    
+    private static final int MAX_CACHE_SIZE = 2; //!!T 512
+
+    private final ConcurrentHashMap<CacheKey, DateFormat> cache = new ConcurrentHashMap<>();
+    private final ConcurrentHashMap<CacheKey, DateFormat> cacheRecallableEntries = new ConcurrentHashMap<>();
+
     private JavaTemplateDateFormatFactory() {
         // Can't be instantiated
     }
@@ -61,68 +62,86 @@ class JavaTemplateDateFormatFactory extends TemplateDateFormatFactory {
 
         // Get DateFormat from global cache:
         CacheKey cacheKey = new CacheKey(dateType, nameOrPattern, locale, timeZone);
-        DateFormat jFormat;
-        
-        jFormat = GLOBAL_FORMAT_CACHE.get(cacheKey);
-        if (jFormat == null) {
-            // Add format to global format cache.
-            StringTokenizer tok = new StringTokenizer(nameOrPattern, "_");
-            int tok1Style = tok.hasMoreTokens() ? parseDateStyleToken(tok.nextToken()) : DateFormat.DEFAULT;
-            if (tok1Style != -1) {
-                switch (dateType) {
-                    case TemplateDateModel.UNKNOWN: {
-                        throw new UnknownDateTypeFormattingUnsupportedException();
-                    }
-                    case TemplateDateModel.TIME: {
-                        jFormat = DateFormat.getTimeInstance(tok1Style, cacheKey.locale);
-                        break;
-                    }
-                    case TemplateDateModel.DATE: {
-                        jFormat = DateFormat.getDateInstance(tok1Style, cacheKey.locale);
-                        break;
-                    }
-                    case TemplateDateModel.DATETIME: {
-                        int tok2Style = tok.hasMoreTokens() ? parseDateStyleToken(tok.nextToken()) : tok1Style;
-                        if (tok2Style != -1) {
-                            jFormat = DateFormat.getDateTimeInstance(tok1Style, tok2Style, cacheKey.locale);
-                        }
-                        break;
-                    }
+
+        DateFormat dateFormat = getFromCache(cacheKey);
+        if (dateFormat == null) {
+            dateFormat = getJavaDateFormatNoCache(dateType, nameOrPattern, cacheKey);
+            dateFormat = addToCacheWithLimitingSize(cacheKey, dateFormat);
+        }
+
+        // Must clone, as SimpleDateFormat is not thread safe, not even if you don't call setters on it:
+        return (DateFormat) dateFormat.clone();
+    }
+
+    private DateFormat addToCacheWithLimitingSize(CacheKey cacheKey, DateFormat dateFormat) {
+        if (cache.size() >= MAX_CACHE_SIZE) {
+            synchronized (JavaTemplateDateFormatFactory.class) {
+                if (cache.size() >= MAX_CACHE_SIZE) {
+                    cacheRecallableEntries.clear();
+                    cacheRecallableEntries.putAll(cache);
+                    cache.clear();
                 }
             }
-            if (jFormat == null) {
-                try {
-                    jFormat = new SimpleDateFormat(nameOrPattern, cacheKey.locale);
-                } catch (IllegalArgumentException e) {
-                    final String msg = e.getMessage();
-                    throw new InvalidFormatParametersException(
-                            msg != null ? msg : "Invalid SimpleDateFormat pattern", e);
+        }
+
+        DateFormat prevDateFormat = cache.putIfAbsent(cacheKey, dateFormat);
+        return prevDateFormat != null ? prevDateFormat : dateFormat;
+    }
+
+    private DateFormat getJavaDateFormatNoCache(int dateType, String nameOrPattern, CacheKey cacheKey) throws
+            UnknownDateTypeFormattingUnsupportedException, InvalidFormatParametersException {
+        DateFormat dateFormat = getJavaDateFormatNoCacheNoCommonAdjustments(dateType, nameOrPattern, cacheKey.locale);
+        dateFormat.setTimeZone(cacheKey.timeZone);
+        return dateFormat;
+    }
+
+    private DateFormat getJavaDateFormatNoCacheNoCommonAdjustments(
+            int dateType, String nameOrPattern, Locale locale)
+            throws UnknownDateTypeFormattingUnsupportedException, InvalidFormatParametersException {
+        StringTokenizer tok = new StringTokenizer(nameOrPattern, "_");
+        int tok1Style = tok.hasMoreTokens() ? parseDateStyleToken(tok.nextToken()) : DateFormat.DEFAULT;
+        if (tok1Style != -1) {
+            switch (dateType) {
+                case TemplateDateModel.UNKNOWN: {
+                    throw new UnknownDateTypeFormattingUnsupportedException();
                 }
-            }
-            jFormat.setTimeZone(cacheKey.timeZone);
-            
-            if (GLOBAL_FORMAT_CACHE.size() >= LEAK_ALERT_DATE_FORMAT_CACHE_SIZE) {
-                boolean triggered = false;
-                synchronized (JavaTemplateDateFormatFactory.class) {
-                    if (GLOBAL_FORMAT_CACHE.size() >= LEAK_ALERT_DATE_FORMAT_CACHE_SIZE) {
-                        triggered = true;
-                        GLOBAL_FORMAT_CACHE.clear();
-                    }
+                case TemplateDateModel.TIME: {
+                    return DateFormat.getTimeInstance(tok1Style, locale);
                 }
-                if (triggered) {
-                    LOG.warn("Global Java DateFormat cache has exceeded " + LEAK_ALERT_DATE_FORMAT_CACHE_SIZE
-                            + " entries => cache flushed. "
-                            + "Typical cause: Some template generates high variety of format pattern strings.");
+                case TemplateDateModel.DATE: {
+                    return DateFormat.getDateInstance(tok1Style, locale);
+                }
+                case TemplateDateModel.DATETIME: {
+                    int tok2Style = tok.hasMoreTokens() ? parseDateStyleToken(tok.nextToken()) : tok1Style;
+                    if (tok2Style != -1) {
+                        return DateFormat.getDateTimeInstance(tok1Style, tok2Style, locale);
+                    }
+                    break;
                 }
             }
-            
-            DateFormat prevJFormat = GLOBAL_FORMAT_CACHE.putIfAbsent(cacheKey, jFormat);
-            if (prevJFormat != null) {
-                jFormat = prevJFormat;
-            }
-        }  // if cache miss
-        
-        return (DateFormat) jFormat.clone();  // For thread safety
+        }
+
+        try {
+            return new SimpleDateFormat(nameOrPattern, locale);
+        } catch (IllegalArgumentException e) {
+            final String msg = e.getMessage();
+            throw new InvalidFormatParametersException(
+                    msg != null ? msg : "Invalid SimpleDateFormat pattern", e);
+        }
+    }
+
+    private DateFormat getFromCache(CacheKey cacheKey) {
+        DateFormat dateFormat = cache.get(cacheKey);
+        if (dateFormat != null) {
+            return dateFormat;
+        }
+
+        dateFormat = cacheRecallableEntries.remove(cacheKey);
+        if (dateFormat == null) {
+            return null;
+        }
+
+        return addToCacheWithLimitingSize(cacheKey, dateFormat);
     }
 
     private static final class CacheKey {
diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml
index b1caabc1..91bc0a89 100644
--- a/src/manual/en_US/book.xml
+++ b/src/manual/en_US/book.xml
@@ -29462,6 +29462,13 @@ TemplateModel x = env.getVariable("x");  // get variable x</programlisting>
               xlink:href="https://github.com/apache/freemarker/pull/82">GitHub
               PR 82</link>)</para>
             </listitem>
+
+            <listitem>
+              <para>Some internal cleanup and slight improves related to
+              global caching of <literal>SimpleDateFormat</literal> patterns
+              (in
+              <literal>freemarker.core.JavaTemplateDateFormatFactory</literal>).</para>
+            </listitem>
           </itemizedlist>
         </section>
       </section>