Skip to content

Commit

Permalink
Allocate ParamStore objects as constants whenever possible.
Browse files Browse the repository at this point in the history 
This is possible because they don't have identity semantics. SoyRecord objects do have identity semantics but not their underlying values.

Share code between `record` `bind` and `call` code paths.

PiperOrigin-RevId: 575886350
  • Loading branch information
@lukesandberg @copybara-github
lukesandberg authored and copybara-github committed Oct 23, 2023
1 parent 08a69cb commit f92f148
Show file tree
Hide file tree
Showing 5 changed files with 161 additions and 137 deletions.
84 changes: 13 additions & 71 deletions java/src/com/google/template/soy/jbcsrc/ExpressionCompiler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,9 @@
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.Future;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -630,83 +632,24 @@ protected void doGen(CodeBuilder adapter) {
});
}

private static final Handle CONSTANT_RECORD_HANDLE =
MethodRef.createPure(
ExtraConstantBootstraps.class,
"constantSoyRecord",
MethodHandles.Lookup.class,
String.class,
Class.class,
int.class,
Object[].class)
.asHandle();

private static final Handle CONSTANT_PARAM_STORE =
MethodRef.createPure(
ExtraConstantBootstraps.class,
"constantParamStore",
MethodHandles.Lookup.class,
String.class,
Class.class,
int.class,
Object[].class)
.asHandle();


@Override
protected SoyExpression visitRecordLiteralNode(RecordLiteralNode node) {
return SoyExpression.forSoyValue(
node.getType(), doVisitRecordLiteralNode(node, /* asParamStore= */ false));
SoyExpression record =
BytecodeUtils.newRecordImplFromParamStore(
node.getType(), node.getSourceLocation(), recordLiteralAsParamStore(node));
return isConstantContext ? record.toMaybeConstant() : record;
}

private Expression doVisitRecordLiteralNode(RecordLiteralNode node, boolean asParamStore) {
int numItems = node.numChildren();
List<Expression> keys = new ArrayList<>(numItems);
List<Expression> values = new ArrayList<>(numItems);
for (int i = 0; i < numItems; i++) {
private Expression recordLiteralAsParamStore(RecordLiteralNode node) {
Map<String, Expression> recordMap = new LinkedHashMap<>();
for (int i = 0; i < node.numChildren(); i++) {
// Keys are RecordProperty objects and values are SoyValue object.
keys.add(BytecodeUtils.constantRecordProperty(node.getKey(i).identifier()));
values.add(visit(node.getChild(i)).box());
}
Expression soyRecord = MethodRefs.PARAM_STORE_SIZE.invoke(constant(keys.size()));
for (int i = 0; i < numItems; i++) {
soyRecord = soyRecord.invoke(MethodRefs.PARAM_STORE_SET_FIELD, keys.get(i), values.get(i));
}
if (!asParamStore) {
soyRecord = MethodRefs.SOY_RECORD_IMPL.invoke(soyRecord);
recordMap.put(node.getKey(i).identifier(), visit(node.getChild(i)));
}
if (isConstantContext && Expression.areAllConstant(values)) {
Object[] constantArgs = new Object[1 + node.numChildren() * 2];
// We store a hash of the source location so that each distinct list authored gets a unique
// value to preserve identity semantics even in constant settings
constantArgs[0] = node.getSourceLocation().hashCode();
for (int i = 0; i < values.size(); i++) {
// We could pass keys.get(i).constantBytecodeValue() but it is more efficient to invoke
// one bulk bootstrap method than a lot of little ones. Caching within `RecordSymbol`
// itself ensures we don't construct duplicate keys.
constantArgs[2 * i + 1] = node.getKey(i).identifier();
constantArgs[2 * i + 2] = values.get(i).constantBytecodeValue();
}
soyRecord =
soyRecord.withConstantValue(
asParamStore
? Expression.ConstantValue.dynamic(
new ConstantDynamic(
"constantParamStore",
BytecodeUtils.PARAM_STORE_TYPE.getDescriptor(),
CONSTANT_PARAM_STORE,
constantArgs),
BytecodeUtils.PARAM_STORE_TYPE,
/* isTrivialConstant= */ false)
: Expression.ConstantValue.dynamic(
new ConstantDynamic(
"constantRecord",
BytecodeUtils.SOY_RECORD_IMPL_TYPE.getDescriptor(),
CONSTANT_RECORD_HANDLE,
constantArgs),
BytecodeUtils.SOY_RECORD_IMPL_TYPE,
/* isTrivialConstant= */ false));
}
return soyRecord;
return BytecodeUtils.newParamStore(Optional.empty(), recordMap);
}

private static final Handle CONSTANT_MAP_HANDLE =
Expand Down Expand Up @@ -1572,8 +1515,7 @@ private SoyExpression visitMethodCall(SoyExpression baseExpr, MethodCallNode nod
node.getType(),
MethodRefs.RUNTIME_BIND_TEMPLATE_PARAMS.invoke(
baseExpr.checkedCast(BytecodeUtils.TEMPLATE_VALUE_TYPE),
doVisitRecordLiteralNode(
(RecordLiteralNode) node.getChild(1), /* asParamStore= */ true)));
recordLiteralAsParamStore((RecordLiteralNode) node.getChild(1))));
}
} else if (function instanceof SoySourceFunctionMethod) {
SoySourceFunctionMethod sourceMethod = (SoySourceFunctionMethod) function;
Expand Down
80 changes: 19 additions & 61 deletions java/src/com/google/template/soy/jbcsrc/SoyNodeCompiler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@
import com.google.template.soy.jbcsrc.restricted.CodeBuilder;
import com.google.template.soy.jbcsrc.restricted.Expression;
import com.google.template.soy.jbcsrc.restricted.Expression.Feature;
import com.google.template.soy.jbcsrc.restricted.FieldRef;
import com.google.template.soy.jbcsrc.restricted.MethodRef;
import com.google.template.soy.jbcsrc.restricted.MethodRefs;
import com.google.template.soy.jbcsrc.restricted.SoyExpression;
Expand Down Expand Up @@ -1728,25 +1727,6 @@ private RecordOrPositional prepareParamsHelper(CallNode node) {
Identifier.create(Names.VARIANT_VAR_NAME, SourceLocation.UNKNOWN),
callBasicNode.getVariantExpr().getRoot()));
}
if (node.numChildren() == 0) {
if (!node.isPassingData()) {
return RecordOrPositional.create(
Suppliers.ofInstance(FieldRef.EMPTY_PARAMS.accessor()), Optional.of(ImmutableMap.of()));
} else if (!node.isPassingAllData()) {
Label reattachLabel = new Label();
return RecordOrPositional.create(
getDataRecordExpression(node, reattachLabel).labelStart(reattachLabel));
}

Expression paramsRecord = parameterLookup.getParamsRecord();
return RecordOrPositional.create(
maybeAddDefaultParams(
node,
MethodRefs.PARAM_STORE_AUGMENT.invoke(paramsRecord, constant(node.numChildren())),
ImmutableMap.of())
.orElse(paramsRecord));
}

ImmutableMap<String, Supplier<Expression>> explicitParams = compileExplicitParams(node);
Supplier<Expression> recordExpression = () -> getParamStoreExpression(node, explicitParams);
return RecordOrPositional.create(
Expand Down Expand Up @@ -1783,56 +1763,33 @@ private ImmutableMap<String, Supplier<Expression>> compileExplicitParams(CallNod
*/
private Expression getParamStoreExpression(
CallNode node, Map<String, Supplier<Expression>> params) {
Expression paramStoreExpression;
if (!node.isPassingData()) {
paramStoreExpression = MethodRefs.PARAM_STORE_SIZE.invoke(constant(params.size()));
} else {
Label reattachDataLabel = new Label();
Expression dataExpression;
if (node.isPassingAllData()) {
dataExpression = parameterLookup.getParamsRecord();
} else {
dataExpression = getDataRecordExpression(node, reattachDataLabel);
}
paramStoreExpression =
MethodRefs.PARAM_STORE_AUGMENT
.invoke(dataExpression, constant(params.size()))
.labelStart(reattachDataLabel);
if (node.isPassingAllData()) {
paramStoreExpression =
maybeAddDefaultParams(node, paramStoreExpression, params).orElse(paramStoreExpression);
}
}
for (var entry : params.entrySet()) {
paramStoreExpression =
MethodRefs.PARAM_STORE_SET_FIELD.invoke(
paramStoreExpression,
BytecodeUtils.constantRecordProperty(entry.getKey()),
entry.getValue().get());
Map<String, Expression> paramsMap = new LinkedHashMap<>();
params.forEach((k, v) -> paramsMap.put(k, v.get()));
if (node.isPassingAllData()) {
maybeAddDefaultParams(node, paramsMap);
}
return paramStoreExpression;

Label reattachDataLabel = new Label();
Optional<Expression> baseRecord =
node.isPassingAllData()
? Optional.of(parameterLookup.getParamsRecord())
: node.isPassingData()
? Optional.of(getDataRecordExpression(node, reattachDataLabel))
: Optional.empty();

return BytecodeUtils.newParamStore(baseRecord, paramsMap).labelStart(reattachDataLabel);
}

private Optional<Expression> maybeAddDefaultParams(
CallNode node,
Expression paramStoreExpression,
Map<String, Supplier<Expression>> explicitParams) {
boolean foundDefaultParams = false;
private void maybeAddDefaultParams(CallNode node, Map<String, Expression> paramsMap) {
// If this is a data="all" call and the caller has default parameters we need to augment the
// params record to make sure any unset default parameters are set to the default in the
// params record. It's not worth it to determine if we're using the default value or not
// here, so just augment all default parameters with whatever value they ended up with.
for (TemplateParam param : node.getNearestAncestor(TemplateNode.class).getParams()) {
if (param.hasDefault() && !explicitParams.containsKey(param.name())) {
foundDefaultParams = true;
paramStoreExpression =
MethodRefs.PARAM_STORE_SET_FIELD.invoke(
paramStoreExpression,
BytecodeUtils.constantRecordProperty(param.name()),
parameterLookup.getParam(param));
if (param.hasDefault() && !paramsMap.containsKey(param.name())) {
paramsMap.put(param.name(), parameterLookup.getParam(param));
}
}
return foundDefaultParams ? Optional.of(paramStoreExpression) : Optional.empty();
}

private Expression getDataRecordExpression(CallNode node, Label reattachPoint) {
Expand All @@ -1841,7 +1798,8 @@ private Expression getDataRecordExpression(CallNode node, Label reattachPoint) {
.compileSubExpression(
node.getDataExpr(), detachState.createExpressionDetacher(reattachPoint))
.box()
.checkedCast(SoyRecord.class));
.checkedCast(SoyRecord.class)
.toMaybeConstant());
}

@Override
Expand Down
108 changes: 108 additions & 0 deletions java/src/com/google/template/soy/jbcsrc/restricted/BytecodeUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.google.template.soy.jbcsrc.restricted;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;

import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
Expand All @@ -32,6 +33,7 @@
import com.google.common.html.types.TrustedResourceUrlProto;
import com.google.common.primitives.Ints;
import com.google.protobuf.Message;
import com.google.template.soy.base.SourceLocation;
import com.google.template.soy.base.internal.SanitizedContentKind;
import com.google.template.soy.data.Dir;
import com.google.template.soy.data.LoggingAdvisingAppendable;
Expand Down Expand Up @@ -244,6 +246,27 @@ public final class BytecodeUtils {
Class.class)
.asHandle();

private static final Handle CONSTANT_PARAM_STORE =
MethodRef.createPure(
ExtraConstantBootstraps.class,
"constantParamStore",
MethodHandles.Lookup.class,
String.class,
Class.class,
Object[].class)
.asHandle();

private static final Handle CONSTANT_RECORD_HANDLE =
MethodRef.createPure(
ExtraConstantBootstraps.class,
"constantSoyRecord",
MethodHandles.Lookup.class,
String.class,
Class.class,
int.class,
Object[].class)
.asHandle();

private static final LoadingCache<Type, Optional<Class<?>>> objectTypeToClassCache =
CacheBuilder.newBuilder()
.build(
Expand Down Expand Up @@ -385,6 +408,8 @@ public static Expression constant(ConstantDynamic value, Features features) {

/** Returns an {@link Expression} that can load the given constant. */
public static Expression constant(Type type, ConstantDynamic value, Features features) {
checkArgument(!value.getName().equals("<init>"));

return new Expression(
type,
// There is no benefit with transforming to an ldc command because we are already there.
Expand Down Expand Up @@ -919,6 +944,89 @@ public static Expression asImmutableList(Iterable<? extends Expression> items) {
Iterables.concat(explicit, ImmutableList.of(remainder)));
}

public static SoyExpression newRecordImplFromParamStore(
SoyType soyType, SourceLocation location, Expression paramStore) {
var recordExp =
SoyExpression.forSoyValue(soyType, MethodRefs.SOY_RECORD_IMPL.invoke(paramStore));
if (paramStore.isConstant()) {
var paramCondy = (ConstantDynamic) paramStore.constantBytecodeValue();
checkState(paramCondy.getBootstrapMethod() == CONSTANT_PARAM_STORE); // sanity check
Object[] args = new Object[1 + paramCondy.getBootstrapMethodArgumentCount()];
// We store a hash of the source location so that each distinct list authored
// gets a unique value to preserve identity semantics even in constant settings
args[0] = location.hashCode();
for (int i = 0; i < paramCondy.getBootstrapMethodArgumentCount(); i++) {
args[i + 1] = paramCondy.getBootstrapMethodArgument(i);
}
// just attach the constant value, it may or may not be appropriate to use it depending
// on the context.
recordExp =
recordExp.withConstantValue(
Expression.ConstantValue.dynamic(
new ConstantDynamic(
"soyRecord",
BytecodeUtils.SOY_RECORD_IMPL_TYPE.getDescriptor(),
CONSTANT_RECORD_HANDLE,
args),
BytecodeUtils.SOY_RECORD_IMPL_TYPE,
/* isTrivialConstant= */ false));
}
return recordExp;
}

/**
* Construct a ParamStore
*
* <p>The values in the params map must either be `SoyValueProvider` expressions or be
* SoyExpression instances that can be trivially coerced to SoyValueProvider
*/
public static Expression newParamStore(
Optional<Expression> baseStore, Map<String, Expression> params) {
baseStore.ifPresent(e -> e.checkAssignableTo(BytecodeUtils.PARAM_STORE_TYPE));
if (params.isEmpty()) {
return baseStore.orElse(FieldRef.EMPTY_PARAMS.accessor());
}
// NOTE: we can always represent
if (Expression.areAllConstant(params.values())
&& baseStore.map(Expression::isConstant).orElse(true)) {
Object[] constantArgs = new Object[params.size() * 2 + (baseStore.isPresent() ? 1 : 0)];
int i = 0;
if (baseStore.isPresent()) {
constantArgs[i++] = baseStore.get().constantBytecodeValue();
}
for (var entry : params.entrySet()) {
// We could pass constantRecordSymbol(entry.getKey()).constantBytecodeValue() but it is more
// efficient to invoke one bulk bootstrap method than a lot of little ones. Caching within
// `RecordSymbol` itself ensures we don't construct duplicate keys.
constantArgs[i++] = entry.getKey();
constantArgs[i++] = entry.getValue().constantBytecodeValue();
}
return constant(
BytecodeUtils.PARAM_STORE_TYPE,
new ConstantDynamic(
"constantParamStore",
BytecodeUtils.PARAM_STORE_TYPE.getDescriptor(),
CONSTANT_PARAM_STORE,
constantArgs),
Features.of(Feature.NON_JAVA_NULLABLE, Feature.NON_SOY_NULLISH));
}
var paramStore =
baseStore.isPresent()
? MethodRefs.PARAM_STORE_AUGMENT.invoke(baseStore.get(), constant(params.size()))
: MethodRefs.PARAM_STORE_SIZE.invoke(constant(params.size()));

for (var entry : params.entrySet()) {
var value = entry.getValue();
if (value instanceof SoyExpression) {
value = ((SoyExpression) value).box();
}
var key = constantRecordProperty(entry.getKey());
paramStore = paramStore.invoke(MethodRefs.PARAM_STORE_SET_FIELD, key, value);
}

return paramStore;
}

public static Expression newImmutableMap(
List<Expression> keys, List<Expression> values, boolean allowDuplicates) {
// We can only call ImmutableList.of if we disallow duplicates or there are 1 or 0 keys
Expand Down
Loading

0 comments on commit f92f148

Please sign in to comment.