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 2016/12/06 22:26:09 UTC

incubator-freemarker git commit: Made `+` operator when adding two hashes significantly faster (be removing the overhead caused be throwing and then catching an exception).

Repository: incubator-freemarker
Updated Branches:
  refs/heads/2.3-gae 496ddfbc0 -> 4b989f89b


Made `+` operator when adding two hashes significantly faster (be removing the overhead caused be throwing and then catching an exception).


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

Branch: refs/heads/2.3-gae
Commit: 4b989f89ba54b0ba309b7385e56e2ecf2466ffef
Parents: 496ddfb
Author: ddekany <dd...@apache.org>
Authored: Tue Dec 6 23:25:42 2016 +0100
Committer: ddekany <dd...@apache.org>
Committed: Tue Dec 6 23:25:55 2016 +0100

----------------------------------------------------------------------
 .../freemarker/core/AddConcatExpression.java    | 58 ++++++++++++++------
 src/main/java/freemarker/core/EvalUtil.java     | 26 +++++++--
 src/manual/en_US/book.xml                       |  4 +-
 3 files changed, 65 insertions(+), 23 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4b989f89/src/main/java/freemarker/core/AddConcatExpression.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/core/AddConcatExpression.java b/src/main/java/freemarker/core/AddConcatExpression.java
index 4dbfd78..5523118 100644
--- a/src/main/java/freemarker/core/AddConcatExpression.java
+++ b/src/main/java/freemarker/core/AddConcatExpression.java
@@ -74,11 +74,26 @@ final class AddConcatExpression extends Expression {
         } else if (leftModel instanceof TemplateSequenceModel && rightModel instanceof TemplateSequenceModel) {
             return new ConcatenatedSequence((TemplateSequenceModel) leftModel, (TemplateSequenceModel) rightModel);
         } else {
+            boolean hashConcatPossible
+                    = leftModel instanceof TemplateHashModel && rightModel instanceof TemplateHashModel;
             try {
+                // We try string addition first. If hash addition is possible, then instead of throwing exception
+                // we return null and do hash addition instead. (We can't simply give hash addition a priority, like
+                // with sequence addition above, as FTL strings are often also FTL hashes.)
                 Object leftOMOrStr = EvalUtil.coerceModelToStringOrMarkup(
-                        leftModel, leftExp, (String) null, env);
+                        leftModel, leftExp, /* returnNullOnNonCoercableType = */ hashConcatPossible, (String) null,
+                        env);
+                if (leftOMOrStr == null) {
+                    return _eval_concatenateHashes(leftModel, rightModel);
+                }
+
+                // Same trick with null return as above.
                 Object rightOMOrStr = EvalUtil.coerceModelToStringOrMarkup(
-                        rightModel, rightExp, (String) null, env);
+                        rightModel, rightExp, /* returnNullOnNonCoercableType = */ hashConcatPossible, (String) null,
+                        env);
+                if (rightOMOrStr == null) {
+                    return _eval_concatenateHashes(leftModel, rightModel);
+                }
 
                 if (leftOMOrStr instanceof String) {
                     if (rightOMOrStr instanceof String) {
@@ -98,25 +113,14 @@ final class AddConcatExpression extends Expression {
                     } else { // rightOMOrStr instanceof TemplateMarkupOutputModel
                         return EvalUtil.concatMarkupOutputs(parent,
                                 leftMO,
-                                (TemplateMarkupOutputModel) rightOMOrStr);
+                                (TemplateMarkupOutputModel<?>) rightOMOrStr);
                     }
                 }
             } catch (NonStringOrTemplateOutputException e) {
-                if (leftModel instanceof TemplateHashModel && rightModel instanceof TemplateHashModel) {
-                    if (leftModel instanceof TemplateHashModelEx && rightModel instanceof TemplateHashModelEx) {
-                        TemplateHashModelEx leftModelEx = (TemplateHashModelEx) leftModel;
-                        TemplateHashModelEx rightModelEx = (TemplateHashModelEx) rightModel;
-                        if (leftModelEx.size() == 0) {
-                            return rightModelEx;
-                        } else if (rightModelEx.size() == 0) {
-                            return leftModelEx;
-                        } else {
-                            return new ConcatenatedHashEx(leftModelEx, rightModelEx);
-                        }
-                    } else {
-                        return new ConcatenatedHash((TemplateHashModel) leftModel,
-                                                    (TemplateHashModel) rightModel);
-                    }
+                // 2.4: Remove this catch; it's for BC, after reworking hash addition so it doesn't rely on this. But
+                // user code might throws this (very unlikely), and then in 2.3.x we did catch that too, incorrectly.
+                if (hashConcatPossible) {
+                    return _eval_concatenateHashes(leftModel, rightModel);
                 } else {
                     throw e;
                 }
@@ -124,6 +128,24 @@ final class AddConcatExpression extends Expression {
         }
     }
 
+    private static TemplateModel _eval_concatenateHashes(TemplateModel leftModel, TemplateModel rightModel)
+            throws TemplateModelException {
+        if (leftModel instanceof TemplateHashModelEx && rightModel instanceof TemplateHashModelEx) {
+            TemplateHashModelEx leftModelEx = (TemplateHashModelEx) leftModel;
+            TemplateHashModelEx rightModelEx = (TemplateHashModelEx) rightModel;
+            if (leftModelEx.size() == 0) {
+                return rightModelEx;
+            } else if (rightModelEx.size() == 0) {
+                return leftModelEx;
+            } else {
+                return new ConcatenatedHashEx(leftModelEx, rightModelEx);
+            }
+        } else {
+            return new ConcatenatedHash((TemplateHashModel) leftModel,
+                                        (TemplateHashModel) rightModel);
+        }
+    }
+
     static TemplateModel _evalOnNumbers(Environment env, TemplateObject parent, Number first, Number second)
             throws TemplateException {
         ArithmeticEngine ae = EvalUtil.getArithmeticEngine(env, parent);

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4b989f89/src/main/java/freemarker/core/EvalUtil.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/core/EvalUtil.java b/src/main/java/freemarker/core/EvalUtil.java
index 886308c..5580a40 100644
--- a/src/main/java/freemarker/core/EvalUtil.java
+++ b/src/main/java/freemarker/core/EvalUtil.java
@@ -27,6 +27,7 @@ import freemarker.template.TemplateBooleanModel;
 import freemarker.template.TemplateCollectionModel;
 import freemarker.template.TemplateDateModel;
 import freemarker.template.TemplateException;
+import freemarker.template.TemplateHashModel;
 import freemarker.template.TemplateModel;
 import freemarker.template.TemplateModelException;
 import freemarker.template.TemplateNumberModel;
@@ -349,9 +350,22 @@ class EvalUtil {
      *            Tip to display if the value type is not coercable, but it's sequence or collection.
      * 
      * @return Never {@code null}
+     * @throws TemplateException 
      */
     static Object coerceModelToStringOrMarkup(TemplateModel tm, Expression exp, String seqTip, Environment env)
             throws TemplateException {
+        return coerceModelToStringOrMarkup(tm, exp, false, seqTip, env);
+    }
+    
+    /**
+     * @return {@code null} if the {@code returnNullOnNonCoercableType} parameter is {@code true}, and the coercion is
+     *         not possible, because of the type is not right for it.
+     * 
+     * @see #coerceModelToStringOrMarkup(TemplateModel, Expression, String, Environment)
+     */
+    static Object coerceModelToStringOrMarkup(
+            TemplateModel tm, Expression exp, boolean returnNullOnNonCoercableType, String seqTip, Environment env)
+            throws TemplateException {
         if (tm instanceof TemplateNumberModel) {
             TemplateNumberModel tnm = (TemplateNumberModel) tm; 
             TemplateNumberFormat format = env.getTemplateNumberFormat(exp, false);
@@ -371,7 +385,7 @@ class EvalUtil {
         } else if (tm instanceof TemplateMarkupOutputModel) {
             return tm;
         } else { 
-            return coerceModelToTextualCommon(tm, exp, seqTip, true, env);
+            return coerceModelToTextualCommon(tm, exp, seqTip, true, returnNullOnNonCoercableType, env);
         }
     }
 
@@ -404,7 +418,7 @@ class EvalUtil {
                 throw MessageUtil.newCantFormatDateException(format, exp, e, false);
             }
         } else { 
-            return coerceModelToTextualCommon(tm, exp, seqTip, false, env);
+            return coerceModelToTextualCommon(tm, exp, seqTip, false, false, env);
         }
     }
 
@@ -424,7 +438,7 @@ class EvalUtil {
         } else if (tm instanceof TemplateDateModel) {
             return assertFormatResultNotNull(env.formatDateToPlainText((TemplateDateModel) tm, exp, false));
         } else {
-            return coerceModelToTextualCommon(tm, exp, seqTip, false, env);
+            return coerceModelToTextualCommon(tm, exp, seqTip, false, false, env);
         }
     }
 
@@ -438,7 +452,8 @@ class EvalUtil {
      * @return Never {@code null}
      */
     private static String coerceModelToTextualCommon(
-            TemplateModel tm, Expression exp, String seqHint, boolean supportsTOM, Environment env)
+            TemplateModel tm, Expression exp, String seqHint, boolean supportsTOM, boolean returnNullOnNonCoercableType,
+            Environment env)
             throws TemplateModelException, InvalidReferenceException, TemplateException,
                     NonStringOrTemplateOutputException, NonStringException {
         if (tm instanceof TemplateScalarModel) {
@@ -481,6 +496,9 @@ class EvalUtil {
             if (env.isClassicCompatible() && tm instanceof BeanModel) {
                 return _BeansAPI.getAsClassicCompatibleString((BeanModel) tm);
             }
+            if (returnNullOnNonCoercableType) {
+                return null;
+            }
             if (seqHint != null && (tm instanceof TemplateSequenceModel || tm instanceof TemplateCollectionModel)) {
                 if (supportsTOM) {
                     throw new NonStringOrTemplateOutputException(exp, tm, seqHint, env);

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/4b989f89/src/manual/en_US/book.xml
----------------------------------------------------------------------
diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml
index 4adf154..6424689 100644
--- a/src/manual/en_US/book.xml
+++ b/src/manual/en_US/book.xml
@@ -26648,7 +26648,9 @@ TemplateModel x = env.getVariable("x");  // get variable x</programlisting>
 
           <itemizedlist>
             <listitem>
-              <para>FIXME</para>
+              <para>Made <literal>+</literal> operator when adding two hashes
+              significantly faster (be removing the overhead caused be
+              throwing and then catching an exception).</para>
             </listitem>
           </itemizedlist>
         </section>