Skip to content

Commit

Permalink
Delete TypedCodecDescriptor, fold it into CodecDescriptor.
Browse files Browse the repository at this point in the history 
When this was originally introduced, the intention was that it would hide
ObjectCodec from callers. That was undone shortly in follow-on CLs. It's no
longer useful.

PiperOrigin-RevId: 601807318
Change-Id: Ifdce155cf324def12627394929ff4ff3f2cd4dc0
  • Loading branch information
@aoeui @copybara-github
aoeui authored and copybara-github committed Jan 26, 2024
1 parent 0db3651 commit 999a26a
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 79 deletions.
7 changes: 3 additions & 4 deletions ...ain/java/com/google/devtools/build/lib/skyframe/serialization/DeserializationContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,19 +67,18 @@ public <T> T deserialize(CodedInputStream codedIn) throws IOException, Serializa
return null;
}
if (tag < 0) {
// Subtract 1 to undo transformation from SerializationContext to avoid null.
// Subtracts 1 to undo transformation from SerializationContext to avoid null.
return (T) deserializer.getMemoized(-tag - 1); // unchecked cast
}
T constant = (T) registry.maybeGetConstantByTag(tag);
if (constant != null) {
return constant;
}
CodecDescriptor codecDescriptor = registry.getCodecDescriptorByTag(tag);
ObjectCodec<T> castCodec = (ObjectCodec<T>) codecDescriptor.getCodec(); // unchecked cast
if (deserializer == null) {
return (T) codecDescriptor.deserialize(this, codedIn); // unchecked cast
return castCodec.deserialize(this, codedIn);
}
@SuppressWarnings("unchecked")
ObjectCodec<T> castCodec = (ObjectCodec<T>) codecDescriptor.getCodec();
return deserializer.deserialize(this, castCodec, codedIn);
}

Expand Down
59 changes: 16 additions & 43 deletions src/main/java/com/google/devtools/build/lib/skyframe/serialization/ObjectCodecRegistry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@

import static com.google.common.collect.ImmutableList.toImmutableList;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import com.google.common.base.Suppliers;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.io.ByteStreams;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.protobuf.CodedInputStream;
import com.google.protobuf.CodedOutputStream;
import com.google.protobuf.MessageLite;
import java.io.IOException;
Expand Down Expand Up @@ -232,12 +232,15 @@ ImmutableList<String> classNames() {
}

/** Describes encoding logic. */
interface CodecDescriptor {
void serialize(SerializationContext context, Object obj, CodedOutputStream codedOut)
throws IOException, SerializationException;
static final class CodecDescriptor {
private final int tag;
private final ObjectCodec<?> codec;

Object deserialize(DeserializationContext context, CodedInputStream codedIn)
throws IOException, SerializationException;
@VisibleForTesting
CodecDescriptor(int tag, ObjectCodec<?> codec) {
this.tag = tag;
this.codec = codec;
}

/**
* Unique identifier for the associated codec.
Expand All @@ -249,41 +252,12 @@ Object deserialize(DeserializationContext context, CodedInputStream codedIn)
* <p>0 is a special tag representing null while negative numbers are reserved for
* backreferences.
*/
int getTag();

/** Returns the underlying codec. */
ObjectCodec<?> getCodec();
}

private static class TypedCodecDescriptor<T> implements CodecDescriptor {
private final int tag;
private final ObjectCodec<T> codec;

private TypedCodecDescriptor(int tag, ObjectCodec<T> codec) {
this.tag = tag;
this.codec = codec;
}

@Override
@SuppressWarnings("unchecked")
public void serialize(SerializationContext context, Object obj, CodedOutputStream codedOut)
throws IOException, SerializationException {
codec.serialize(context, (T) obj, codedOut);
}

@Override
public T deserialize(DeserializationContext context, CodedInputStream codedIn)
throws IOException, SerializationException {
return codec.deserialize(context, codedIn);
}

@Override
public int getTag() {
return tag;
}

@Override
public ObjectCodec<T> getCodec() {
/** Returns the underlying codec. */
public ObjectCodec<?> getCodec() {
return codec;
}

Expand Down Expand Up @@ -398,7 +372,7 @@ private static int processCodecs(
for (ObjectCodec<?> codec :
ImmutableList.sortedCopyOf(
Comparator.comparing(o -> o.getEncodedClass().getName()), memoizingCodecs)) {
CodecDescriptor codecDescriptor = new TypedCodecDescriptor<>(nextTag, codec);
CodecDescriptor codecDescriptor = new CodecDescriptor(nextTag, codec);
addToChecksum(checksum, nextTag, codec.getClass().getName());
tagMappedCodecsBuilder.add(codecDescriptor);
codecsBuilder.put(codec.getEncodedClass(), codecDescriptor);
Expand Down Expand Up @@ -460,7 +434,7 @@ private static CodecDescriptor createDynamicCodecDescriptor(int tag, String clas
if (MessageLite.class.isAssignableFrom(type)) {
return createCodecDescriptorForProto(tag, type);
}
return new TypedCodecDescriptor<>(tag, new DynamicCodec(type));
return new CodecDescriptor(tag, new DynamicCodec(type));
} catch (ReflectiveOperationException e) {
new SerializationException("Could not create codec for type: " + className, e)
.printStackTrace();
Expand All @@ -470,13 +444,12 @@ private static CodecDescriptor createDynamicCodecDescriptor(int tag, String clas

@SuppressWarnings({"unchecked", "rawtypes"})
private static CodecDescriptor createCodecDescriptorForEnum(int tag, Class<?> enumType) {
return new TypedCodecDescriptor(tag, new EnumCodec(enumType));
return new CodecDescriptor(tag, new EnumCodec(enumType));
}

@SuppressWarnings({"unchecked", "rawtypes"})
@SuppressWarnings("unchecked")
private static CodecDescriptor createCodecDescriptorForProto(int tag, Class<?> protoType) {
return new TypedCodecDescriptor(
tag, new MessageLiteCodec((Class<? extends MessageLite>) protoType));
return new CodecDescriptor(tag, new MessageLiteCodec((Class<? extends MessageLite>) protoType));
}

private CodecDescriptor getDynamicCodecDescriptor(String className, @Nullable Class<?> type)
Expand Down
17 changes: 9 additions & 8 deletions src/main/java/com/google/devtools/build/lib/skyframe/serialization/SerializationContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,16 @@ public void serialize(Object object, CodedOutputStream codedOut)
throws IOException, SerializationException {
ObjectCodecRegistry.CodecDescriptor descriptor =
recordAndGetDescriptorIfNotConstantMemoizedOrNull(object, codedOut);
if (descriptor != null) {
if (serializer == null) {
descriptor.serialize(this, object, codedOut);
} else {
@SuppressWarnings("unchecked")
ObjectCodec<Object> castCodec = (ObjectCodec<Object>) descriptor.getCodec();
serializer.serialize(this, object, castCodec, codedOut);
}
if (descriptor == null) {
return;
}
@SuppressWarnings("unchecked")
ObjectCodec<Object> castCodec = (ObjectCodec<Object>) descriptor.getCodec();
if (serializer == null) {
castCodec.serialize(this, object, codedOut);
return;
}
serializer.serialize(this, object, castCodec, codedOut);
}

@Override
Expand Down
25 changes: 1 addition & 24 deletions ...java/com/google/devtools/build/lib/skyframe/serialization/DeserializationContextTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
Expand Down Expand Up @@ -63,26 +62,6 @@ public void constantDeserialize() throws Exception {
verify(registry).maybeGetConstantByTag(1);
}

@Test
public void descriptorDeserialize() throws Exception {
ObjectCodecRegistry.CodecDescriptor codecDescriptor =
mock(ObjectCodecRegistry.CodecDescriptor.class);
ObjectCodecRegistry registry = mock(ObjectCodecRegistry.class);
when(registry.getCodecDescriptorByTag(1)).thenReturn(codecDescriptor);
CodedInputStream codedInputStream = mock(CodedInputStream.class);
when(codedInputStream.readSInt32()).thenReturn(1);
DeserializationContext deserializationContext =
new DeserializationContext(registry, ImmutableClassToInstanceMap.of());
Object returnValue = new Object();
when(codecDescriptor.deserialize(deserializationContext, codedInputStream))
.thenReturn(returnValue);
assertThat((Object) deserializationContext.deserialize(codedInputStream))
.isSameInstanceAs(returnValue);
verify(codedInputStream).readSInt32();
verify(registry).getCodecDescriptorByTag(1);
verify(codecDescriptor).deserialize(deserializationContext, codedInputStream);
}

@Test
public void memoizingDeserialize_null() throws SerializationException, IOException {
ObjectCodecRegistry registry = mock(ObjectCodecRegistry.class);
Expand Down Expand Up @@ -120,8 +99,7 @@ public void memoizingDeserialize_codec() throws SerializationException, IOExcept
when(codec.getEncodedClass()).thenAnswer(unused -> Object.class);
when(codec.additionalEncodedClasses()).thenReturn(ImmutableList.of());
ObjectCodecRegistry.CodecDescriptor codecDescriptor =
mock(ObjectCodecRegistry.CodecDescriptor.class);
doReturn(codec).when(codecDescriptor).getCodec();
new ObjectCodecRegistry.CodecDescriptor(/* tag= */ 1, codec);
ObjectCodecRegistry registry = mock(ObjectCodecRegistry.class);
when(registry.getCodecDescriptorByTag(1)).thenReturn(codecDescriptor);
CodedInputStream codedInputStream = mock(CodedInputStream.class);
Expand All @@ -134,7 +112,6 @@ public void memoizingDeserialize_codec() throws SerializationException, IOExcept
verify(codedInputStream).readSInt32();
verify(registry).maybeGetConstantByTag(1);
verify(registry).getCodecDescriptorByTag(1);
verify(codecDescriptor).getCodec();
verify(codec).deserialize(deserializationContext, codedInputStream);
}

Expand Down

0 comments on commit 999a26a

Please sign in to comment.