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/26 00:05:10 UTC

[freemarker] branch FREEMARKER-35 updated (70a1bbcd -> 0a233022)

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

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


    from 70a1bbcd FREEMARKER-35: Some very minimal documentation in the version history.
     new 4c8eeca8 FREEMARKER-35: Noted that at least Java 8 in required now in the version history.
     new 0a233022 Continued reworked factory-level caches: Fixing/cleaning previous commit in JavaTemplateDateFormatFactory. Factored out caching logic into its own class. Using that in JavaTemplateNumberFormatFactory too now.

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../java/freemarker/core/FastLRUKeyValueStore.java | 85 ++++++++++++++++++++++
 .../core/JavaTemplateDateFormatFactory.java        | 55 ++------------
 .../core/JavaTemplateNumberFormatFactory.java      | 79 +++++++-------------
 src/manual/en_US/book.xml                          | 12 ++-
 4 files changed, 127 insertions(+), 104 deletions(-)
 create mode 100644 src/main/java/freemarker/core/FastLRUKeyValueStore.java


[freemarker] 01/02: FREEMARKER-35: Noted that at least Java 8 in required now in the version history.

Posted by dd...@apache.org.
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 4c8eeca8b71eb8474900167544f2d706f23ea2a6
Author: ddekany <dd...@apache.org>
AuthorDate: Fri Aug 26 02:01:50 2022 +0200

    FREEMARKER-35: Noted that at least Java 8 in required now in the version history.
---
 src/manual/en_US/book.xml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml
index 08527762..d8264608 100644
--- a/src/manual/en_US/book.xml
+++ b/src/manual/en_US/book.xml
@@ -29417,6 +29417,9 @@ TemplateModel x = env.getVariable("x");  // get variable x</programlisting>
 
         <para>Release date: TODO</para>
 
+        <para>Please note that with this version the minimum required Java
+        version was increased from Java 7 to Java 8.</para>
+
         <section>
           <title>Changes on the FTL side</title>
 


[freemarker] 02/02: Continued reworked factory-level caches: Fixing/cleaning previous commit in JavaTemplateDateFormatFactory. Factored out caching logic into its own class. Using that in JavaTemplateNumberFormatFactory too now.

Posted by dd...@apache.org.
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 0a233022d031e76605cb6ff195d2f2d54f868022
Author: ddekany <dd...@apache.org>
AuthorDate: Fri Aug 26 02:04:24 2022 +0200

    Continued reworked factory-level caches: Fixing/cleaning previous commit in JavaTemplateDateFormatFactory. Factored out caching logic into its own class. Using that in JavaTemplateNumberFormatFactory too now.
---
 .../java/freemarker/core/FastLRUKeyValueStore.java | 85 ++++++++++++++++++++++
 .../core/JavaTemplateDateFormatFactory.java        | 55 ++------------
 .../core/JavaTemplateNumberFormatFactory.java      | 79 +++++++-------------
 src/manual/en_US/book.xml                          |  9 ++-
 4 files changed, 124 insertions(+), 104 deletions(-)

diff --git a/src/main/java/freemarker/core/FastLRUKeyValueStore.java b/src/main/java/freemarker/core/FastLRUKeyValueStore.java
new file mode 100644
index 00000000..db774162
--- /dev/null
+++ b/src/main/java/freemarker/core/FastLRUKeyValueStore.java
@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, 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.
+ */
+
+package freemarker.core;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * A thread-safe {@link Map}-like object, with get/put-like operations, that implements a rough, but fast
+ * least-recently-used (LRU) logic to remove old entries automatically, in order to keep the size under a specified
+ * maximum. It will remember at least the last {@link #guaranteedRecentEntries} accessed entries. Removing entries is
+ * only guaranteed when the size exceeds the double of {@link #guaranteedRecentEntries}. Actually, if the methods are
+ * accessed from N threads concurrently, there's a chance to end up with N-1 more remembered entries, though in
+ * practical applications this is very unlikely to happen. That's also precision given up for speed.
+ *
+ * @since 2.3.32
+ */
+class FastLRUKeyValueStore<K, V> {
+    private final int guaranteedRecentEntries;
+
+    private final ConcurrentHashMap<K, V> recentEntries = new ConcurrentHashMap<>();
+    private final ConcurrentHashMap<K, V> olderEntries = new ConcurrentHashMap<>();
+
+    /**
+     * @param guaranteedRecentEntries
+     *         The number of least recently accessed ("get", or "put") entries that we are guaranteed to remember. The
+     *         actual size can grow bigger than that; see in class documentation.
+     */
+    public FastLRUKeyValueStore(int guaranteedRecentEntries) {
+        this.guaranteedRecentEntries = guaranteedRecentEntries;
+    }
+
+    /**
+     * @return If the value was already in the cache, then it's not replaced, and the old value is returned, otherwise
+     * the new (argument) value is returned.
+     */
+    V putIfAbsentThenReturnStored(K key, V value) {
+        if (recentEntries.size() >= guaranteedRecentEntries) {
+            synchronized (this) {
+                if (recentEntries.size() >= guaranteedRecentEntries) {
+                    olderEntries.clear();
+                    olderEntries.putAll(recentEntries);
+                    recentEntries.clear();
+                }
+            }
+        }
+
+        V prevValue = recentEntries.putIfAbsent(key, value);
+        return prevValue != null ? prevValue : value;
+    }
+
+    /**
+     * @return {@code null} if there's no entry with the given key.
+     */
+    V get(K cacheKey) {
+        V value = recentEntries.get(cacheKey);
+        if (value != null) {
+            return value;
+        }
+
+        value = olderEntries.remove(cacheKey);
+        if (value == null) {
+            return null;
+        }
+
+        return putIfAbsentThenReturnStored(cacheKey, value);
+    }
+}
diff --git a/src/main/java/freemarker/core/JavaTemplateDateFormatFactory.java b/src/main/java/freemarker/core/JavaTemplateDateFormatFactory.java
index 62863c2e..f4773ac3 100644
--- a/src/main/java/freemarker/core/JavaTemplateDateFormatFactory.java
+++ b/src/main/java/freemarker/core/JavaTemplateDateFormatFactory.java
@@ -24,7 +24,6 @@ import java.text.SimpleDateFormat;
 import java.util.Locale;
 import java.util.StringTokenizer;
 import java.util.TimeZone;
-import java.util.concurrent.ConcurrentHashMap;
 
 import freemarker.log.Logger;
 import freemarker.template.TemplateDateModel;
@@ -35,10 +34,7 @@ class JavaTemplateDateFormatFactory extends TemplateDateFormatFactory {
     
     private static final Logger LOG = Logger.getLogger("freemarker.runtime");
 
-    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 final FastLRUKeyValueStore<CacheKey, DateFormat> dateFormatCache = new FastLRUKeyValueStore<>(512);
 
     private JavaTemplateDateFormatFactory() {
         // Can't be instantiated
@@ -51,41 +47,16 @@ class JavaTemplateDateFormatFactory extends TemplateDateFormatFactory {
     @Override
     public TemplateDateFormat get(String params, int dateType, Locale locale, TimeZone timeZone, boolean zonelessInput,
             Environment env) throws UnknownDateTypeFormattingUnsupportedException, InvalidFormatParametersException {
-        return new JavaTemplateDateFormat(getJavaDateFormat(dateType, params, locale, timeZone));
-    }
-
-    /**
-     * Returns a "private" copy (not in the global cache) for the given format.  
-     */
-    private DateFormat getJavaDateFormat(int dateType, String nameOrPattern, Locale locale, TimeZone timeZone)
-            throws UnknownDateTypeFormattingUnsupportedException, InvalidFormatParametersException {
-
-        // Get DateFormat from global cache:
-        CacheKey cacheKey = new CacheKey(dateType, nameOrPattern, locale, timeZone);
+        CacheKey cacheKey = new CacheKey(dateType, params, locale, timeZone);
 
-        DateFormat dateFormat = getFromCache(cacheKey);
+        DateFormat dateFormat = dateFormatCache.get(cacheKey);
         if (dateFormat == null) {
-            dateFormat = getJavaDateFormatNoCache(dateType, nameOrPattern, cacheKey);
-            dateFormat = addToCacheWithLimitingSize(cacheKey, dateFormat);
+            dateFormat = getJavaDateFormatNoCache(dateType, params, cacheKey);
+            dateFormat = dateFormatCache.putIfAbsentThenReturnStored(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();
-                }
-            }
-        }
-
-        DateFormat prevDateFormat = cache.putIfAbsent(cacheKey, dateFormat);
-        return prevDateFormat != null ? prevDateFormat : dateFormat;
+        return new JavaTemplateDateFormat((DateFormat) dateFormat.clone());
     }
 
     private DateFormat getJavaDateFormatNoCache(int dateType, String nameOrPattern, CacheKey cacheKey) throws
@@ -130,20 +101,6 @@ class JavaTemplateDateFormatFactory extends TemplateDateFormatFactory {
         }
     }
 
-    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 {
         private final int dateType;
         private final String pattern;
diff --git a/src/main/java/freemarker/core/JavaTemplateNumberFormatFactory.java b/src/main/java/freemarker/core/JavaTemplateNumberFormatFactory.java
index 8e8a3279..a3de36a3 100644
--- a/src/main/java/freemarker/core/JavaTemplateNumberFormatFactory.java
+++ b/src/main/java/freemarker/core/JavaTemplateNumberFormatFactory.java
@@ -21,9 +21,6 @@ package freemarker.core;
 import java.text.NumberFormat;
 import java.text.ParseException;
 import java.util.Locale;
-import java.util.concurrent.ConcurrentHashMap;
-
-import freemarker.log.Logger;
 
 /**
  * Deals with {@link TemplateNumberFormat}-s that just wrap a Java {@link NumberFormat}.
@@ -34,11 +31,7 @@ class JavaTemplateNumberFormatFactory extends TemplateNumberFormatFactory {
 
     static final String COMPUTER = "computer";
 
-    private static final Logger LOG = Logger.getLogger("freemarker.runtime");
-
-    private static final ConcurrentHashMap<CacheKey, NumberFormat> GLOBAL_FORMAT_CACHE
-            = new ConcurrentHashMap<>();
-    private static final int LEAK_ALERT_NUMBER_FORMAT_CACHE_SIZE = 1024;
+    private final FastLRUKeyValueStore<CacheKey, NumberFormat> numberFormatCache = new FastLRUKeyValueStore<>(512);
 
     private JavaTemplateNumberFormatFactory() {
         // Not meant to be instantiated
@@ -50,51 +43,35 @@ class JavaTemplateNumberFormatFactory extends TemplateNumberFormatFactory {
         CacheKey cacheKey = new CacheKey(
                 env != null ? env.transformNumberFormatGlobalCacheKey(params) : params,
                 locale);
-        NumberFormat jFormat = GLOBAL_FORMAT_CACHE.get(cacheKey);
-        if (jFormat == null) {
-            if ("number".equals(params)) {
-                jFormat = NumberFormat.getNumberInstance(locale);
-            } else if ("currency".equals(params)) {
-                jFormat = NumberFormat.getCurrencyInstance(locale);
-            } else if ("percent".equals(params)) {
-                jFormat = NumberFormat.getPercentInstance(locale);
-            } else if (COMPUTER.equals(params)) {
-                jFormat = env.getCNumberFormat();
-            } else {
-                try {
-                    jFormat = ExtendedDecimalFormatParser.parse(params, locale);
-                } catch (ParseException e) {
-                    String msg = e.getMessage();
-                    throw new InvalidFormatParametersException(
-                            msg != null ? msg : "Invalid DecimalFormat pattern", e);
-                }
-            }
 
-            if (GLOBAL_FORMAT_CACHE.size() >= LEAK_ALERT_NUMBER_FORMAT_CACHE_SIZE) {
-                boolean triggered = false;
-                synchronized (JavaTemplateNumberFormatFactory.class) {
-                    if (GLOBAL_FORMAT_CACHE.size() >= LEAK_ALERT_NUMBER_FORMAT_CACHE_SIZE) {
-                        triggered = true;
-                        GLOBAL_FORMAT_CACHE.clear();
-                    }
-                }
-                if (triggered) {
-                    LOG.warn("Global Java NumberFormat cache has exceeded " + LEAK_ALERT_NUMBER_FORMAT_CACHE_SIZE
-                            + " entries => cache flushed. "
-                            + "Typical cause: Some template generates high variety of format pattern strings.");
-                }
-            }
-            
-            NumberFormat prevJFormat = GLOBAL_FORMAT_CACHE.putIfAbsent(cacheKey, jFormat);
-            if (prevJFormat != null) {
-                jFormat = prevJFormat;
-            }
-        }  // if cache miss
-        
-        // JFormat-s aren't thread-safe; must clone it
-        jFormat = (NumberFormat) jFormat.clone();
+        NumberFormat numberFormat = numberFormatCache.get(cacheKey);
+        if (numberFormat == null) {
+            numberFormat = getNumberFormatNoCache(params, locale, env);
+            numberFormat = numberFormatCache.putIfAbsentThenReturnStored(cacheKey, numberFormat);
+        }
         
-        return new JavaTemplateNumberFormat(jFormat, params); 
+        return new JavaTemplateNumberFormat((NumberFormat) numberFormat.clone(), params);
+    }
+
+    private NumberFormat getNumberFormatNoCache(String params, Locale locale, Environment env) throws
+            InvalidFormatParametersException {
+        if ("number".equals(params)) {
+            return NumberFormat.getNumberInstance(locale);
+        } else if ("currency".equals(params)) {
+            return NumberFormat.getCurrencyInstance(locale);
+        } else if ("percent".equals(params)) {
+            return NumberFormat.getPercentInstance(locale);
+        } else if (COMPUTER.equals(params)) {
+            return env.getCNumberFormat();
+        } else {
+            try {
+                return ExtendedDecimalFormatParser.parse(params, locale);
+            } catch (ParseException e) {
+                String msg = e.getMessage();
+                throw new InvalidFormatParametersException(
+                        msg != null ? msg : "Invalid DecimalFormat pattern", e);
+            }
+        }
     }
 
     private static final class CacheKey {
diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml
index d8264608..d6fc9270 100644
--- a/src/manual/en_US/book.xml
+++ b/src/manual/en_US/book.xml
@@ -29515,10 +29515,11 @@ TemplateModel x = env.getVariable("x");  // get variable x</programlisting>
             </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>
+              <para>Some internal cleanup, and slightly improved global
+              caching of <literal>SimpleDateFormat</literal>, and
+              <literal>DecimalFormat</literal> patterns (in
+              <literal>freemarker.core.JavaTemplateDateFormatFactory</literal>,
+              and <literal>JavaTemplateNumberFormatFactory</literal>).</para>
             </listitem>
           </itemizedlist>
         </section>