Skip to content

Commit

Permalink
Make ProtoStringInterner static.
Browse files Browse the repository at this point in the history
Requiring an instance to be created is not necessary and makes using it a little more awkward in some cases. We never expect to use it with an interner other than that provided by `StringInterner` anyway.

PiperOrigin-RevId: 613156938
  • Loading branch information
Googler authored and copybara-github committed Mar 6, 2024
1 parent c3246cc commit be97d08
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.collect.Multimaps.flatteningToMultimap;
import static com.google.idea.blaze.common.proto.ProtoStringInterner.intern;
import static java.util.Objects.requireNonNull;

import com.google.auto.value.AutoValue;
Expand All @@ -34,7 +35,6 @@
import com.google.devtools.build.lib.query2.proto.proto2api.Build;
import com.google.devtools.build.lib.query2.proto.proto2api.Build.Target;
import com.google.idea.blaze.common.Label;
import com.google.idea.blaze.common.proto.ProtoStringInterner;
import com.google.idea.blaze.qsync.query.Query.SourceFile;
import java.io.BufferedInputStream;
import java.io.File;
Expand Down Expand Up @@ -136,8 +136,7 @@ public static QuerySummary create(InputStream protoInputStream) throws IOExcepti
Map<String, Query.Rule> ruleMap = Maps.newHashMap();
Set<String> packagesWithErrors = Sets.newHashSet();
Build.Target target;
ProtoStringInterner interner = new ProtoStringInterner();
while ((target = interner.intern(Target.parseDelimitedFrom(protoInputStream))) != null) {
while ((target = intern(Target.parseDelimitedFrom(protoInputStream))) != null) {
switch (target.getType()) {
case SOURCE_FILE:
Query.SourceFile sourceFile =
Expand Down Expand Up @@ -337,8 +336,7 @@ public Builder putAllPackagesWithErrors(Set<Path> packagesWithErrors) {
}

public QuerySummary build() {
ProtoStringInterner interner = new ProtoStringInterner();
return QuerySummary.create(interner.intern(builder.build()));
return QuerySummary.create(intern(builder.build()));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,24 +27,17 @@
/**
* Eliminates duplicate strings inside a proto message by interning them.
*
* <p>All fields in a proto message are examined and strings passed through a {@link Interner} to
* ensure that identical strings use the same instance in memory. This then allows duplicated
* <p>All fields in a proto message are examined and strings passed through {@link StringInterner}
* to ensure that identical strings use the same instance in memory. This then allows duplicated
* strings to be garbage collected.
*/
public class ProtoStringInterner {

private final Interner<String> stringInterner;

public ProtoStringInterner(Interner<String> interner) {
this.stringInterner = interner;
}

public ProtoStringInterner() {
this(StringInterner.INSTANCE);
}
private ProtoStringInterner() {}

@SuppressWarnings("unchecked") // casts of proto field values should be safe.
public <T extends Message> T intern(T message) {
public static <T extends Message> T intern(T message) {
Interner<String> stringInterner = StringInterner.INSTANCE;
if (message == null) {
return null;
}
Expand All @@ -68,7 +61,7 @@ public <T extends Message> T intern(T message) {
if (field.isRepeated()) {
List<Message> originalList = (List<Message>) original;
List<Message> internedList =
originalList.stream().map(this::intern).collect(toImmutableList());
originalList.stream().map(ProtoStringInterner::intern).collect(toImmutableList());
if (!IntStream.range(0, internedList.size())
.allMatch(i -> internedList.get(i) == originalList.get(i))) {
interned = internedList;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,32 +47,28 @@ public void sanity_check() throws ParseException {

@Test
public void basic_string() throws ParseException {
ProtoStringInterner interner = new ProtoStringInterner();
MyMessage message1 = interner.intern(parse("string: 'value'"));
MyMessage message2 = interner.intern(parse("string: 'value'"));
MyMessage message1 = ProtoStringInterner.intern(parse("string: 'value'"));
MyMessage message2 = ProtoStringInterner.intern(parse("string: 'value'"));
assertThat(message1.getString()).isSameInstanceAs(message2.getString());
}

@Test
public void no_strings_message_not_copied() throws ParseException {
ProtoStringInterner interner = new ProtoStringInterner();
MyMessage original = parse("integer: 42");
MyMessage interned = interner.intern(original);
MyMessage interned = ProtoStringInterner.intern(original);
assertThat(interned).isSameInstanceAs(original);
}

@Test
public void repeated_strings() throws ParseException {
ProtoStringInterner interner = new ProtoStringInterner();
MyMessage original = parse("strings: 'one'", "strings: 'one'", "strings: 'two'");
MyMessage interned = interner.intern(original);
MyMessage interned = ProtoStringInterner.intern(original);
assertThat(interned).isEqualTo(original);
assertThat(interned.getStrings(0)).isSameInstanceAs(interned.getStrings(1));
}

@Test
public void string_map_values() throws ParseException {
ProtoStringInterner interner = new ProtoStringInterner();
MyMessage original =
parse(
"string_map {",
Expand All @@ -88,40 +84,37 @@ public void string_map_values() throws ParseException {
" value: 'puppies'",
"}");

MyMessage interned = interner.intern(original);
MyMessage interned = ProtoStringInterner.intern(original);
assertThat(interned).isEqualTo(original);
assertThat(interned.getStringMapMap().get("one"))
.isSameInstanceAs(interned.getStringMapMap().get("two"));
}

@Test
public void string_map_keys() throws ParseException {
ProtoStringInterner interner = new ProtoStringInterner();
MyMessage original =
parse("string: 'one'", "string_map {", " key: 'one'", " value: 'kittens'", "}");

MyMessage interned = interner.intern(original);
MyMessage interned = ProtoStringInterner.intern(original);
assertThat(interned).isEqualTo(original);
assertThat(interned.getString())
.isSameInstanceAs(Iterables.getOnlyElement(interned.getStringMapMap().keySet()));
}

@Test
public void nested_string() throws ParseException {
ProtoStringInterner interner = new ProtoStringInterner();
MyMessage original = parse("string: 'hello'", "sub_message {", " sub_string: 'hello'", "}");

MyMessage interned = interner.intern(original);
MyMessage interned = ProtoStringInterner.intern(original);
assertThat(interned).isEqualTo(original);
assertThat(interned.getString()).isSameInstanceAs(interned.getSubMessage().getSubString());
}

@Test
public void nested_repeated_field_no_strings_not_copied() throws ParseException {
ProtoStringInterner interner = new ProtoStringInterner();
MyMessage original =
parse("sub_messages {", " sub_int: 1", "}", "sub_messages {", " sub_int: 2", "}");
MyMessage interned = interner.intern(original);
MyMessage interned = ProtoStringInterner.intern(original);
assertThat(interned).isSameInstanceAs(original);
}
}

0 comments on commit be97d08

Please sign in to comment.