diff --git a/common/src/main/java/dev/cel/common/values/ProtoLiteCelValueConverter.java b/common/src/main/java/dev/cel/common/values/ProtoLiteCelValueConverter.java index 932bec658..c4b9dacaa 100644 --- a/common/src/main/java/dev/cel/common/values/ProtoLiteCelValueConverter.java +++ b/common/src/main/java/dev/cel/common/values/ProtoLiteCelValueConverter.java @@ -464,20 +464,29 @@ boolean hasFieldByNumber(ByteString bytes, String protoTypeName, SelectField fie return false; } - /** Describes {@code field} by the type information it carries. */ + /** Describes {@code field} by the type information it carries, built once and kept on it. */ private static FieldLiteDescriptor newFieldDescriptor(SelectField field) { + Object cached = field.liteFieldDescriptor(); + if (cached != null) { + return (FieldLiteDescriptor) cached; + } // Only presence tests omit the type code: their presence check doesn't depend on the type, and // the fields they navigate through are messages, as are map entries. boolean isMap = field.mapEntrySpec() != null; boolean isMessage = isMap || field.typeCode() == SelectField.NO_TYPE_CODE; - return newFieldDescriptor( - field.fieldNumber(), - field.fieldName(), - isMap - ? EncodingType.MAP - : (field.defaultValue() instanceof List ? EncodingType.LIST : EncodingType.SINGULAR), - isMessage ? SelectField.MESSAGE_TYPE_CODE : field.typeCode(), - field.protoTypeName()); + FieldLiteDescriptor fieldDescriptor = + newFieldDescriptor( + field.fieldNumber(), + field.fieldName(), + isMap + ? EncodingType.MAP + : (field.defaultValue() instanceof List + ? EncodingType.LIST + : EncodingType.SINGULAR), + isMessage ? SelectField.MESSAGE_TYPE_CODE : field.typeCode(), + field.protoTypeName()); + field.setLiteFieldDescriptor(fieldDescriptor); + return fieldDescriptor; } private static FieldLiteDescriptor newFieldDescriptor( diff --git a/common/src/main/java/dev/cel/common/values/RawProtoMessageLiteValue.java b/common/src/main/java/dev/cel/common/values/RawProtoMessageLiteValue.java index 8a34751cf..03847d576 100644 --- a/common/src/main/java/dev/cel/common/values/RawProtoMessageLiteValue.java +++ b/common/src/main/java/dev/cel/common/values/RawProtoMessageLiteValue.java @@ -46,7 +46,7 @@ abstract class RawProtoMessageLiteValue extends StructValue findByFieldNumber(SelectField field) { - return protoLiteCelValueConverter().findByFieldNumber(toByteString(), celType().name(), field); + return protoLiteCelValueConverter().findByFieldNumber(toByteString(), protoTypeName(), field); } static RawProtoMessageLiteValue create( @@ -158,8 +158,7 @@ static RawProtoMessageLiteValue create( checkNotNull(protoLiteCelValueConverter); return new AutoValue_RawProtoMessageLiteValue( rawWireBytes, - StructTypeReference.create( - protoTypeName.isEmpty() ? UNKNOWN_MESSAGE_TYPE_NAME : protoTypeName), + protoTypeName.isEmpty() ? UNKNOWN_MESSAGE_TYPE_NAME : protoTypeName, protoLiteCelValueConverter, isView); } diff --git a/common/src/main/java/dev/cel/common/values/SelectField.java b/common/src/main/java/dev/cel/common/values/SelectField.java index 6df39c37c..c883d315f 100644 --- a/common/src/main/java/dev/cel/common/values/SelectField.java +++ b/common/src/main/java/dev/cel/common/values/SelectField.java @@ -32,7 +32,8 @@ @AutoValue @AutoValue.CopyAnnotations @Immutable -@SuppressWarnings("Immutable") // Default value is an immutable CEL literal or null +// Default value is an immutable CEL literal or null; liteFieldDescriptor is a memo of this field. +@SuppressWarnings("Immutable") public abstract class SelectField { private static final int MAX_FIELD_NUMBER = 536870911; @@ -83,6 +84,19 @@ public abstract class SelectField { */ abstract @Nullable MapEntrySpec mapEntrySpec(); + // The lite runtime's FieldLiteDescriptor for this field, built from this field alone on first + // use so descriptorless hops don't rebuild it. Typed Object to keep this target free of protobuf + // deps. A race at most rebuilds it, as the descriptor's fields are all final. + private @Nullable Object liteFieldDescriptor; + + @Nullable Object liteFieldDescriptor() { + return liteFieldDescriptor; + } + + void setLiteFieldDescriptor(Object liteFieldDescriptor) { + this.liteFieldDescriptor = liteFieldDescriptor; + } + /** * Creates a presence-test qualifier hop. * diff --git a/common/src/test/java/dev/cel/common/values/RawProtoMessageLiteValueTest.java b/common/src/test/java/dev/cel/common/values/RawProtoMessageLiteValueTest.java index 932ff42f3..f47eef35f 100644 --- a/common/src/test/java/dev/cel/common/values/RawProtoMessageLiteValueTest.java +++ b/common/src/test/java/dev/cel/common/values/RawProtoMessageLiteValueTest.java @@ -277,6 +277,20 @@ public void selectByFieldNumber_presentOnWire_decoded() throws Exception { assertThat(val).isEqualTo("hello"); } + @Test + public void selectByFieldNumber_twice_reusesFieldDescriptor() throws Exception { + RawProtoMessageLiteValue raw = + RawProtoMessageLiteValue.create(encode(out -> out.writeInt64(1, 42L)), EMPTY_CONVERTER); + SelectField field = SelectField.create(1L, "single_int64", 3, 0L); + raw.selectByFieldNumber(field); + Object fieldDescriptor = field.liteFieldDescriptor(); + + raw.selectByFieldNumber(field); + + assertThat(fieldDescriptor).isNotNull(); + assertThat(field.liteFieldDescriptor()).isSameInstanceAs(fieldDescriptor); + } + @Test public void selectByFieldNumber_absentWithDefaultValue_returnsDefault() { RawProtoMessageLiteValue raw =