Skip to content

Commit 0e0c298

Browse files
TAKETODAYsbrannen
authored andcommitted
Optimize InlineMap and InlineList in SpEL
- Make InlineList#constant and InlineMap#constant final. - Add more assertions for InlineMap tests. Closes gh-30251
1 parent baf3678 commit 0e0c298

File tree

3 files changed

+62
-60
lines changed

3 files changed

+62
-60
lines changed

spring-expression/src/main/java/org/springframework/expression/spel/ast/InlineList.java

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -35,55 +35,57 @@
3535
*
3636
* @author Andy Clement
3737
* @author Sam Brannen
38+
* @author Harry Yang
3839
* @since 3.0.4
3940
*/
4041
public class InlineList extends SpelNodeImpl {
4142

4243
// If the list is purely literals, it is a constant value and can be computed and cached
4344
@Nullable
44-
private TypedValue constant; // TODO must be immutable list
45+
private final TypedValue constant;
4546

4647

4748
public InlineList(int startPos, int endPos, SpelNodeImpl... args) {
4849
super(startPos, endPos, args);
49-
checkIfConstant();
50+
this.constant = computeConstantValue();
5051
}
5152

5253

5354
/**
54-
* If all the components of the list are constants, or lists that themselves contain constants, then a constant list
55-
* can be built to represent this node. This will speed up later getValue calls and reduce the amount of garbage
55+
* If all the components of the list are constants, or lists
56+
* that themselves contain constants, then a constant list
57+
* can be built to represent this node. This will speed up
58+
* later getValue calls and reduce the amount of garbage
5659
* created.
5760
*/
58-
private void checkIfConstant() {
59-
boolean isConstant = true;
61+
@Nullable
62+
private TypedValue computeConstantValue() {
6063
for (int c = 0, max = getChildCount(); c < max; c++) {
6164
SpelNode child = getChild(c);
6265
if (!(child instanceof Literal)) {
6366
if (child instanceof InlineList inlineList) {
6467
if (!inlineList.isConstant()) {
65-
isConstant = false;
68+
return null;
6669
}
6770
}
6871
else {
69-
isConstant = false;
72+
return null;
7073
}
7174
}
7275
}
73-
if (isConstant) {
74-
List<Object> constantList = new ArrayList<>();
75-
int childcount = getChildCount();
76-
for (int c = 0; c < childcount; c++) {
77-
SpelNode child = getChild(c);
78-
if (child instanceof Literal literal) {
79-
constantList.add(literal.getLiteralValue().getValue());
80-
}
81-
else if (child instanceof InlineList inlineList) {
82-
constantList.add(inlineList.getConstantValue());
83-
}
76+
77+
List<Object> constantList = new ArrayList<>();
78+
int childcount = getChildCount();
79+
for (int c = 0; c < childcount; c++) {
80+
SpelNode child = getChild(c);
81+
if (child instanceof Literal literal) {
82+
constantList.add(literal.getLiteralValue().getValue());
83+
}
84+
else if (child instanceof InlineList inlineList) {
85+
constantList.add(inlineList.getConstantValue());
8486
}
85-
this.constant = new TypedValue(Collections.unmodifiableList(constantList));
8687
}
88+
return new TypedValue(Collections.unmodifiableList(constantList));
8789
}
8890

8991
@Override

spring-expression/src/main/java/org/springframework/expression/spel/ast/InlineMap.java

Lines changed: 36 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -32,18 +32,19 @@
3232
*
3333
* @author Andy Clement
3434
* @author Sam Brannen
35+
* @author Harry Yang
3536
* @since 4.1
3637
*/
3738
public class InlineMap extends SpelNodeImpl {
3839

3940
// If the map is purely literals, it is a constant value and can be computed and cached
4041
@Nullable
41-
private TypedValue constant;
42+
private final TypedValue constant;
4243

4344

4445
public InlineMap(int startPos, int endPos, SpelNodeImpl... args) {
4546
super(startPos, endPos, args);
46-
checkIfConstant();
47+
this.constant = computeConstantValue();
4748
}
4849

4950

@@ -52,59 +53,55 @@ public InlineMap(int startPos, int endPos, SpelNodeImpl... args) {
5253
* contain constants, then a constant list can be built to represent this node.
5354
* This will speed up later getValue calls and reduce the amount of garbage created.
5455
*/
55-
private void checkIfConstant() {
56-
boolean isConstant = true;
56+
@Nullable
57+
private TypedValue computeConstantValue() {
5758
for (int c = 0, max = getChildCount(); c < max; c++) {
5859
SpelNode child = getChild(c);
5960
if (!(child instanceof Literal)) {
6061
if (child instanceof InlineList inlineList) {
6162
if (!inlineList.isConstant()) {
62-
isConstant = false;
63-
break;
63+
return null;
6464
}
6565
}
6666
else if (child instanceof InlineMap inlineMap) {
6767
if (!inlineMap.isConstant()) {
68-
isConstant = false;
69-
break;
68+
return null;
7069
}
7170
}
7271
else if (!(c % 2 == 0 && child instanceof PropertyOrFieldReference)) {
73-
isConstant = false;
74-
break;
72+
return null;
7573
}
7674
}
7775
}
78-
if (isConstant) {
79-
Map<Object, Object> constantMap = new LinkedHashMap<>();
80-
int childCount = getChildCount();
81-
for (int c = 0; c < childCount; c++) {
82-
SpelNode keyChild = getChild(c++);
83-
SpelNode valueChild = getChild(c);
84-
Object key = null;
85-
Object value = null;
86-
if (keyChild instanceof Literal literal) {
87-
key = literal.getLiteralValue().getValue();
88-
}
89-
else if (keyChild instanceof PropertyOrFieldReference propertyOrFieldReference) {
90-
key = propertyOrFieldReference.getName();
91-
}
92-
else {
93-
return;
94-
}
95-
if (valueChild instanceof Literal literal) {
96-
value = literal.getLiteralValue().getValue();
97-
}
98-
else if (valueChild instanceof InlineList inlineList) {
99-
value = inlineList.getConstantValue();
100-
}
101-
else if (valueChild instanceof InlineMap inlineMap) {
102-
value = inlineMap.getConstantValue();
103-
}
104-
constantMap.put(key, value);
76+
77+
Map<Object, Object> constantMap = new LinkedHashMap<>();
78+
int childCount = getChildCount();
79+
for (int c = 0; c < childCount; c++) {
80+
SpelNode keyChild = getChild(c++);
81+
SpelNode valueChild = getChild(c);
82+
Object key;
83+
Object value = null;
84+
if (keyChild instanceof Literal literal) {
85+
key = literal.getLiteralValue().getValue();
86+
}
87+
else if (keyChild instanceof PropertyOrFieldReference propertyOrFieldReference) {
88+
key = propertyOrFieldReference.getName();
89+
}
90+
else {
91+
return null;
92+
}
93+
if (valueChild instanceof Literal literal) {
94+
value = literal.getLiteralValue().getValue();
95+
}
96+
else if (valueChild instanceof InlineList inlineList) {
97+
value = inlineList.getConstantValue();
98+
}
99+
else if (valueChild instanceof InlineMap inlineMap) {
100+
value = inlineMap.getConstantValue();
105101
}
106-
this.constant = new TypedValue(Collections.unmodifiableMap(constantMap));
102+
constantMap.put(key, value);
107103
}
104+
return new TypedValue(Collections.unmodifiableMap(constantMap));
108105
}
109106

110107
@Override

spring-expression/src/test/java/org/springframework/expression/spel/MapTests.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,9 @@ public void testConstantRepresentation1() {
117117
checkConstantMap("{#root.name:true}",false);
118118
checkConstantMap("{a:1,b:2,c:{d:true,e:false}}", true);
119119
checkConstantMap("{a:1,b:2,c:{d:{1,2,3},e:{4,5,6},f:{'a','b','c'}}}", true);
120+
// for nested InlineMap
121+
checkConstantMap("{a:{k:#d}}", false);
122+
checkConstantMap("{@bean:@bean}", false);
120123
}
121124

122125
private void checkConstantMap(String expressionText, boolean expectedToBeConstant) {

0 commit comments

Comments
 (0)