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 2019/03/31 22:34:03 UTC
[freemarker] 02/03: Made ?size smarter when its result is compared
to an integer literal. Specifically,
for ?filter-ed sequences (or for the result of similar future
Stream-API-like built-ins) it won't count more elements then it's necessary
to tell the comparison result. Also, for a TemplateCollectionModelEx,
it calls isEmpty() instead of size(),
if for the comparison result we only have to know if we have more than 0
elements.
This is an automated email from the ASF dual-hosted git repository.
ddekany pushed a commit to branch 2.3-gae
in repository https://gitbox.apache.org/repos/asf/freemarker.git
commit e7d91e63596c97a59568af78467f1854b3b11a2f
Author: ddekany <dd...@apache.org>
AuthorDate: Sun Mar 31 18:17:04 2019 +0200
Made ?size smarter when its result is compared to an integer literal. Specifically, for ?filter-ed sequences (or for the result of similar future Stream-API-like built-ins) it won't count more elements then it's necessary to tell the comparison result. Also, for a TemplateCollectionModelEx, it calls isEmpty() instead of size(), if for the comparison result we only have to know if we have more than 0 elements.
---
src/main/java/freemarker/core/BuiltIn.java | 5 +-
.../freemarker/core/BuiltInsForMultipleTypes.java | 37 +++++++++++-
.../java/freemarker/core/ComparisonExpression.java | 16 +++++
src/main/java/freemarker/core/EvalUtil.java | 12 ++++
src/main/java/freemarker/core/IteratorBlock.java | 6 +-
src/main/java/freemarker/core/MiscUtil.java | 7 +++
src/manual/en_US/book.xml | 15 +++++
...ilyGeneratedSeqTargetSupportInBuiltinsTest.java | 69 +++++++++++++++++++++-
8 files changed, 158 insertions(+), 9 deletions(-)
diff --git a/src/main/java/freemarker/core/BuiltIn.java b/src/main/java/freemarker/core/BuiltIn.java
index 0679e21..9e3dd3c 100644
--- a/src/main/java/freemarker/core/BuiltIn.java
+++ b/src/main/java/freemarker/core/BuiltIn.java
@@ -388,8 +388,9 @@ abstract class BuiltIn extends Expression implements Cloneable {
bi.key = key;
bi.target = target;
if (bi.isLazilyGeneratedSequenceModelTargetSupported()) {
- if (target instanceof BuiltInsForSequences.IntermediateStreamOperationLikeBuiltIn) {
- ((BuiltInsForSequences.IntermediateStreamOperationLikeBuiltIn) target)
+ Expression cleanedTarget = MiscUtil.peelParentheses(target);
+ if (cleanedTarget instanceof BuiltInsForSequences.IntermediateStreamOperationLikeBuiltIn) {
+ ((BuiltInsForSequences.IntermediateStreamOperationLikeBuiltIn) cleanedTarget)
.setLazyResultGenerationAllowed(true);
}
}
diff --git a/src/main/java/freemarker/core/BuiltInsForMultipleTypes.java b/src/main/java/freemarker/core/BuiltInsForMultipleTypes.java
index 61f8f3f..b690cf5 100644
--- a/src/main/java/freemarker/core/BuiltInsForMultipleTypes.java
+++ b/src/main/java/freemarker/core/BuiltInsForMultipleTypes.java
@@ -48,6 +48,7 @@ import freemarker.template.TemplateScalarModel;
import freemarker.template.TemplateSequenceModel;
import freemarker.template.TemplateTransformModel;
import freemarker.template._TemplateAPI;
+import freemarker.template.utility.NumberUtil;
/**
* A holder for builtins that didn't fit into any other category.
@@ -488,12 +489,16 @@ class BuiltInsForMultipleTypes {
return true;
}
+ private int countingLimit;
+
@Override
TemplateModel _eval(Environment env) throws TemplateException {
TemplateModel model = target.eval(env);
final int size;
- if (model instanceof TemplateSequenceModel) {
+ if (countingLimit == 1 && model instanceof TemplateCollectionModelEx) {
+ size = ((TemplateCollectionModelEx) model).isEmpty() ? 0 : 1;
+ } else if (model instanceof TemplateSequenceModel) {
size = ((TemplateSequenceModel) model).size();
} else if (model instanceof TemplateCollectionModelEx) {
size = ((TemplateCollectionModelEx) model).size();
@@ -508,8 +513,11 @@ class BuiltInsForMultipleTypes {
// expected by the template author, and we just made that calculation less wasteful here.
TemplateModelIterator iterator = ((LazilyGeneratedSequenceModel) model).iterator();
int counter = 0;
- while (iterator.hasNext()) {
+ countElements: while (iterator.hasNext()) {
counter++;
+ if (counter == countingLimit) {
+ break countElements;
+ }
iterator.next();
}
size = counter;
@@ -526,6 +534,31 @@ class BuiltInsForMultipleTypes {
}
return new SimpleNumber(size);
}
+
+ /**
+ * Enables an optimization trick when the result of the built-in will be compared with a number literal.
+ * For example, in the case of {@code things?size != 0} we only need to check of the target is non-empty, which
+ * is often more efficient than telling exactly how many elements it has.
+ */
+ void setCountingLimit(int cmpOperator, NumberLiteral rightOperand) {
+ int cmpInt;
+ try {
+ cmpInt = NumberUtil.toIntExact(rightOperand.getAsNumber());
+ } catch (ArithmeticException e) {
+ // As we can't know what the ArithmeticEngine will be on runtime, we won't risk this for non-integers.
+ return;
+ }
+
+ switch (cmpOperator) {
+ case EvalUtil.CMP_OP_EQUALS: countingLimit = cmpInt + 1; break;
+ case EvalUtil.CMP_OP_NOT_EQUALS: countingLimit = cmpInt + 1; break;
+ case EvalUtil.CMP_OP_LESS_THAN: countingLimit = cmpInt; break;
+ case EvalUtil.CMP_OP_GREATER_THAN: countingLimit = cmpInt + 1; break;
+ case EvalUtil.CMP_OP_LESS_THAN_EQUALS: countingLimit = cmpInt + 1; break;
+ case EvalUtil.CMP_OP_GREATER_THAN_EQUALS: countingLimit = cmpInt; break;
+ default: throw new BugException("Unsupported comparator operator code: " + cmpOperator);
+ }
+ }
}
static class stringBI extends BuiltIn {
diff --git a/src/main/java/freemarker/core/ComparisonExpression.java b/src/main/java/freemarker/core/ComparisonExpression.java
index f647ce4..b33c5bc 100644
--- a/src/main/java/freemarker/core/ComparisonExpression.java
+++ b/src/main/java/freemarker/core/ComparisonExpression.java
@@ -51,6 +51,22 @@ final class ComparisonExpression extends BooleanExpression {
} else {
throw new BugException("Unknown comparison operator " + opString);
}
+
+ Expression cleanedLeft = MiscUtil.peelParentheses(left);
+ Expression cleanedRight = MiscUtil.peelParentheses(right);
+ if (cleanedLeft instanceof BuiltInsForMultipleTypes.sizeBI) {
+ if (cleanedRight instanceof NumberLiteral) {
+ ((BuiltInsForMultipleTypes.sizeBI) cleanedLeft).setCountingLimit(
+ operation, (NumberLiteral) cleanedRight
+ );
+ }
+ } else if (cleanedRight instanceof BuiltInsForMultipleTypes.sizeBI) {
+ if (cleanedLeft instanceof NumberLiteral) {
+ ((BuiltInsForMultipleTypes.sizeBI) cleanedRight).setCountingLimit(
+ EvalUtil.mirrorCmpOperator(operation), (NumberLiteral) cleanedLeft
+ );
+ }
+ }
}
/*
diff --git a/src/main/java/freemarker/core/EvalUtil.java b/src/main/java/freemarker/core/EvalUtil.java
index ebde9c3..0fa7a48 100644
--- a/src/main/java/freemarker/core/EvalUtil.java
+++ b/src/main/java/freemarker/core/EvalUtil.java
@@ -343,6 +343,18 @@ class EvalUtil {
}
}
+ static int mirrorCmpOperator(int operator) {
+ switch (operator) {
+ case CMP_OP_EQUALS: return CMP_OP_EQUALS;
+ case CMP_OP_NOT_EQUALS: return CMP_OP_NOT_EQUALS;
+ case CMP_OP_LESS_THAN: return CMP_OP_GREATER_THAN;
+ case CMP_OP_GREATER_THAN: return CMP_OP_LESS_THAN;
+ case CMP_OP_LESS_THAN_EQUALS: return CMP_OP_GREATER_THAN_EQUALS;
+ case CMP_OP_GREATER_THAN_EQUALS: return CMP_OP_LESS_THAN_EQUALS;
+ default: throw new BugException("Unsupported comparator operator code: " + operator);
+ }
+ }
+
/**
* Converts a value to plain text {@link String}, or a {@link TemplateMarkupOutputModel} if that's what the
* {@link TemplateValueFormat} involved produces.
diff --git a/src/main/java/freemarker/core/IteratorBlock.java b/src/main/java/freemarker/core/IteratorBlock.java
index a51401a..c734cbe 100644
--- a/src/main/java/freemarker/core/IteratorBlock.java
+++ b/src/main/java/freemarker/core/IteratorBlock.java
@@ -83,8 +83,10 @@ final class IteratorBlock extends TemplateElement {
this.hashListing = hashListing;
this.forEach = forEach;
- if (listedExp instanceof BuiltInsForSequences.IntermediateStreamOperationLikeBuiltIn) {
- ((BuiltInsForSequences.IntermediateStreamOperationLikeBuiltIn) listedExp).setLazyResultGenerationAllowed(true);
+ Expression cleanedListExp = MiscUtil.peelParentheses(listedExp);
+ if (cleanedListExp instanceof BuiltInsForSequences.IntermediateStreamOperationLikeBuiltIn) {
+ ((BuiltInsForSequences.IntermediateStreamOperationLikeBuiltIn) cleanedListExp)
+ .setLazyResultGenerationAllowed(true);
fetchElementsOutsideLoopVarContext = true;
} else {
fetchElementsOutsideLoopVarContext = false;
diff --git a/src/main/java/freemarker/core/MiscUtil.java b/src/main/java/freemarker/core/MiscUtil.java
index 770e394..505edb4 100644
--- a/src/main/java/freemarker/core/MiscUtil.java
+++ b/src/main/java/freemarker/core/MiscUtil.java
@@ -64,5 +64,12 @@ class MiscUtil {
});
return res;
}
+
+ static Expression peelParentheses(Expression exp) {
+ while (exp instanceof ParentheticalExpression) {
+ exp = ((ParentheticalExpression) exp).getNestedExpression();
+ }
+ return exp;
+ }
}
diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml
index 0633247..24ceada 100644
--- a/src/manual/en_US/book.xml
+++ b/src/manual/en_US/book.xml
@@ -27963,6 +27963,21 @@ TemplateModel x = env.getVariable("x"); // get variable x</programlisting>
</listitem>
<listitem>
+ <para>If the result of
+ <literal><replaceable>seq</replaceable>?size</literal> is
+ compared to an integer literal in a template, like in
+ <literal><replaceable>seq</replaceable>?size != 0</literal>, or
+ <literal><replaceable>seq</replaceable>?size < 1</literal>,
+ and to decide the answer it's enough to know if
+ <literal><replaceable>seq</replaceable></literal> is an empty
+ sequence or collection (i.e., the exact size isn't needed), and
+ said value implements
+ <literal>TemplateCollectionModelEx</literal>, FreeMarker will
+ call <literal>TemplateCollectionModelEx.isEmpty()</literal>
+ instead of <literal>size()</literal>.</para>
+ </listitem>
+
+ <listitem>
<para>Added
<literal>TemplateModelUtils.wrapAsHashUnion(ObjectWrapper,
List<?>)</literal> and
diff --git a/src/test/java/freemarker/core/LazilyGeneratedSeqTargetSupportInBuiltinsTest.java b/src/test/java/freemarker/core/LazilyGeneratedSeqTargetSupportInBuiltinsTest.java
index 37cefa8..36af95f 100644
--- a/src/test/java/freemarker/core/LazilyGeneratedSeqTargetSupportInBuiltinsTest.java
+++ b/src/test/java/freemarker/core/LazilyGeneratedSeqTargetSupportInBuiltinsTest.java
@@ -43,7 +43,7 @@ import freemarker.test.TemplateTest;
public class LazilyGeneratedSeqTargetSupportInBuiltinsTest extends TemplateTest {
@Test
- public void sizeTest() throws Exception {
+ public void sizeBasicsTest() throws Exception {
assertOutput("${seq?size}",
"[size]3");
assertOutput("${collEx?size}",
@@ -63,6 +63,56 @@ public class LazilyGeneratedSeqTargetSupportInBuiltinsTest extends TemplateTest
}
@Test
+ public void sizeComparisonTest() throws Exception {
+ // These actually aren't related to lazy generation...
+ assertOutput("${collEx?size}",
+ "[size]3");
+ assertOutput("${collEx?size != 0}",
+ "[isEmpty]true");
+ assertOutput("${0 != collEx?size}",
+ "[isEmpty]true");
+ assertOutput("${collEx?size == 0}",
+ "[isEmpty]false");
+ assertOutput("${0 == collEx?size}",
+ "[isEmpty]false");
+ assertOutput("${(collEx?size >= 1)}",
+ "[isEmpty]true");
+ assertOutput("${1 <= collEx?size}",
+ "[isEmpty]true");
+ assertOutput("${collEx?size <= 0}",
+ "[isEmpty]false");
+ assertOutput("${(0 >= collEx?size)}",
+ "[isEmpty]false");
+ assertOutput("${collEx?size > 0}",
+ "[isEmpty]true");
+ assertOutput("${0 < collEx?size}",
+ "[isEmpty]true");
+ assertOutput("${collEx?size < 1}",
+ "[isEmpty]false");
+ assertOutput("${1 > collEx?size}",
+ "[isEmpty]false");
+ assertOutput("${collEx?size == 1}",
+ "[size]false");
+ assertOutput("${1 == collEx?size}",
+ "[size]false");
+
+ // Now the lazy generation things:
+ assertOutput("${collLong?filter(x -> true)?size}",
+ "[iterator]" +
+ "[hasNext][next][hasNext][next][hasNext][next]" +
+ "[hasNext][next][hasNext][next][hasNext][next][hasNext]6");
+ // Note: "[next]" is added by ?filter, as it has to know if the element matches the predicate.
+ assertOutput("${collLong?filter(x -> true)?size != 0}",
+ "[iterator][hasNext][next]true");
+ assertOutput("${collLong?filter(x -> true)?size != 1}",
+ "[iterator][hasNext][next][hasNext][next]true");
+ assertOutput("${collLong?filter(x -> true)?size == 1}",
+ "[iterator][hasNext][next][hasNext][next]false");
+ assertOutput("${collLong?filter(x -> true)?size < 3}",
+ "[iterator][hasNext][next][hasNext][next][hasNext][next]false");
+ }
+
+ @Test
public void firstTest() throws Exception {
assertOutput("${coll?first}",
"[iterator][hasNext][next]1");
@@ -87,13 +137,26 @@ public class LazilyGeneratedSeqTargetSupportInBuiltinsTest extends TemplateTest
@Test
public void listTest() throws Exception {
// Note: #list has to fetch elements up to 4 to know if it?hasNext
- assertOutput("<#list collLong?filter(x -> x % 2 == 0) as it>${it} ${it?hasNext?c}<#break></#list>",
- "[iterator][hasNext][next][hasNext][next][hasNext][next][hasNext][next]2 true");
+ String commonSourceExp = "collLong?filter(x -> x % 2 == 0)";
+ for (String sourceExp : new String[] {commonSourceExp, "(" + commonSourceExp + ")"}) {
+ assertOutput("<#list " + sourceExp + " as it>${it} ${it?hasNext}<#break></#list>",
+ "[iterator][hasNext][next][hasNext][next][hasNext][next][hasNext][next]2 true");
+ }
}
+ @Test
+ public void biTargetParenthesisTest() throws Exception {
+ assertOutput("${(coll?filter(x -> x % 2 == 0))?first}",
+ "[iterator][hasNext][next][hasNext][next]2");
+ }
+
+
+
@Override
protected Configuration createConfiguration() throws Exception {
Configuration cfg = super.createConfiguration();
+ cfg.setIncompatibleImprovements(Configuration.VERSION_2_3_29);
+ cfg.setBooleanFormat("c");
cfg.setSharedVariable("seq", new MonitoredTemplateSequenceModel(1, 2, 3));
cfg.setSharedVariable("coll", new MonitoredTemplateCollectionModel(1, 2, 3));
cfg.setSharedVariable("collLong", new MonitoredTemplateCollectionModel(1, 2, 3, 4, 5, 6));