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 Commit: 4b989f89ba54b0ba309b7385e56e2ecf2466ffef Parents: 496ddfb Author: ddekany <[email protected]> Authored: Tue Dec 6 23:25:42 2016 +0100 Committer: ddekany <[email protected]> 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>
