freemarker-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ddek...@apache.org
Subject incubator-freemarker git commit: Clarified buffering concerns regarding TemplateLoaderResult.getReader and getInputStream.
Date Sat, 11 Feb 2017 16:20:28 GMT
Repository: incubator-freemarker
Updated Branches:
  refs/heads/3 3d0fdc8c7 -> a612ce0db


Clarified buffering concerns regarding TemplateLoaderResult.getReader and getInputStream.


Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/a612ce0d
Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/a612ce0d
Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/a612ce0d

Branch: refs/heads/3
Commit: a612ce0db53f8551d646bacf83f179d7b4acecb8
Parents: 3d0fdc8
Author: ddekany <ddekany@apache.org>
Authored: Sat Feb 11 17:20:15 2017 +0100
Committer: ddekany <ddekany@apache.org>
Committed: Sat Feb 11 17:20:15 2017 +0100

----------------------------------------------------------------------
 src/main/java/freemarker/cache/TemplateCache.java   |  8 +++++---
 .../freemarker/cache/TemplateLoadingResult.java     | 16 ++++++++++++----
 src/main/java/freemarker/template/Template.java     | 10 ++++++++--
 3 files changed, 25 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/a612ce0d/src/main/java/freemarker/cache/TemplateCache.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/cache/TemplateCache.java b/src/main/java/freemarker/cache/TemplateCache.java
index d545986..9439b78 100644
--- a/src/main/java/freemarker/cache/TemplateCache.java
+++ b/src/main/java/freemarker/cache/TemplateCache.java
@@ -628,12 +628,14 @@ public class TemplateCache {
                     if (!inputStream.markSupported()) {
                         inputStream = new BufferedInputStream(inputStream);
                     }
-                    inputStream.mark(Integer.MAX_VALUE); // [FM3] Mark should be released
after the 1st FTL tag
-                    templateSpecifiedEncodingHandler = new MarkReleaserTemplateSpecifiedEncodingHandler(
-                            inputStream);
+                    inputStream.mark(Integer.MAX_VALUE); // Mark is released after the 1st
FTL tag
+                    templateSpecifiedEncodingHandler = new MarkReleaserTemplateSpecifiedEncodingHandler(inputStream);
                 } else {
                     templateSpecifiedEncodingHandler = null; 
                 }
+                // Regarding buffering worries: On the Reader side we should only read in
chunks (like through a
+                // BufferedReader), so there shouldn't be a problem if the InputStream is
not buffered. (Also, at least
+                // on Oracle JDK and OpenJDK 7 the InputStreamReader itself has an internal
~8K buffer.)
                 reader = new InputStreamReader(inputStream, initialEncoding);
             } else {
                 throw new IllegalStateException("For a(n) " + templateLoaderResult.getClass().getName()

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/a612ce0d/src/main/java/freemarker/cache/TemplateLoadingResult.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/cache/TemplateLoadingResult.java b/src/main/java/freemarker/cache/TemplateLoadingResult.java
index 87dcac9..3bc7f8d 100644
--- a/src/main/java/freemarker/cache/TemplateLoadingResult.java
+++ b/src/main/java/freemarker/cache/TemplateLoadingResult.java
@@ -18,6 +18,8 @@
  */
 package freemarker.cache;
 
+import java.io.BufferedInputStream;
+import java.io.BufferedReader;
 import java.io.InputStream;
 import java.io.Reader;
 import java.io.Serializable;
@@ -56,7 +58,8 @@ public final class TemplateLoadingResult {
      *            See {@link #getVersion()} for the meaning of this. Can be {@code null},
but use that only if the
      *            backing storage mechanism doesn't know this information.
      * @param reader
-     *            Gives the content of the template
+     *            Gives the content of the template. It will be read in few thousand character
chunks by FreeMarker, so
+     *            generally it need not be a {@link BufferedReader}.
      * @param templateConfiguration
      *            Usually {@code null}, as usually the backing storage mechanism doesn't
store such information;
      *            see {@link #getTemplateConfiguration()}.
@@ -85,7 +88,8 @@ public final class TemplateLoadingResult {
      *            See {@link #getVersion()} for the meaning of this. Can be {@code null},
but use that only if the
      *            backing storage mechanism doesn't know this information.
      * @param inputStream
-     *            Gives the content of the template
+     *            Gives the content of the template. It will be read in few thousand byte
chunks by FreeMarker, so
+     *            generally it need not be a {@link BufferedInputStream}.
      * @param templateConfiguration
      *            Usually {@code null}, as usually the backing storage mechanism doesn't
store such information; see
      *            {@link #getTemplateConfiguration()}. The most probable application is supplying
the charset (encoding)
@@ -124,8 +128,8 @@ public final class TemplateLoadingResult {
      * The return value is always the same instance, no mater when and how many times this
method is called.
      * 
      * <p>
-     * The return {@code InputStream} should use buffering if that's useful considering the
backing storage mechanism.
-     * TODO Is it really needed?
+     * The returned {@code InputStream} will be read in few kilobyte chunks by FreeMarker,
so generally it need not
+     * be a {@link BufferedInputStream}. 
      * 
      * @return {@code null} or a {@code InputStream} to read the template content; see method
description for more.
      */
@@ -172,6 +176,10 @@ public final class TemplateLoadingResult {
      * responsibility of the caller (which is {@link TemplateCache} usually) to {@code close()}
the {@link Reader}. The
      * return value is always the same instance, no mater when and how many times this method
is called.
      * 
+     * <p>
+     * The returned {@code Reader} will be read in few thousand character chunks by FreeMarker,
so generally it need not
+     * be a {@link BufferedReader}. 
+     * 
      * @return {@code null} or a {@code Reader} to read the template content; see method
description for more.
      */
     public Reader getReader() {

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/a612ce0d/src/main/java/freemarker/template/Template.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/template/Template.java b/src/main/java/freemarker/template/Template.java
index ac9bde2..42b7ee9 100644
--- a/src/main/java/freemarker/template/Template.java
+++ b/src/main/java/freemarker/template/Template.java
@@ -80,6 +80,8 @@ import freemarker.template.utility.NullArgumentException;
 public class Template extends Configurable {
     public static final String DEFAULT_NAMESPACE_PREFIX = "D";
     public static final String NO_NS_PREFIX = "N";
+
+    private static final int READER_BUFFER_SIZE = 8192;
     
     /** This is only non-null during parsing. It's used internally to make some information
available through the
      *  Template API-s earlier than the parsing was finished. */
@@ -168,7 +170,8 @@ public class Template extends Configurable {
      * @param reader
      *            The character stream to read from. The {@link Reader} is <em>not</em>
closed by this method (unlike
      *            in FreeMarker 2.x.x), so be sure that it's closed somewhere. (Except of
course, readers like
-     *            {@link StringReader} need not be closed.)
+     *            {@link StringReader} need not be closed.) The {@link Reader} need not be
buffered, because this
+     *            method ensures that it will be read in few kilobyte chunks, not byte by
byte.
      * @param cfg
      *            The Configuration object that this Template is associated with. If this
is {@code null}, the "default"
      *            {@link Configuration} object is used, which is highly discouraged, because
it can easily lead to
@@ -254,9 +257,12 @@ public class Template extends Configurable {
         try {
             ParserConfiguration actualParserConfiguration = getParserConfiguration();
             
+            // Ensure that the parameter Reader is only read in bigger chunks, as we don't
know if the it's buffered.
+            // In particular, inside the FreeMarker code, we assume that the stream stages
need not be buffered.
             if (!(reader instanceof BufferedReader) && !(reader instanceof StringReader))
{
-                reader = new BufferedReader(reader, 0x1000);
+                reader = new BufferedReader(reader, READER_BUFFER_SIZE);
             }
+            
             ltbReader = new LineTableBuilder(reader, actualParserConfiguration);
             reader = ltbReader;
             


Mime
View raw message