Skip to content

Commit

Permalink
In builders, track unset primitive properties with bitmasks rather th…
Browse files Browse the repository at this point in the history
…an boxing.

Previously, an AutoValue property of type `int` was tracked by a field of type `Integer` in the builder. This was solely so that we could use `null` to indicate that the property had not been set. It meant that the memory footprint of a builder that sets primitive properties was bigger than you would expect.

AutoValue actually used a bitset for the same purposes in early versions. That was changed because GWT doesn't support `java.util.BitSet`. It does now, but anyway we now use primitive integer fields for the bitset since that is faster and more compact.

RELNOTES=AutoValue and AutoBuilder builders now use bitmasks to track unset primitive properties, which will typically lead to smaller builder objects and faster build times.
PiperOrigin-RevId: 420374999
  • Loading branch information
eamonnmcmanus authored and Google Java Core Libraries committed Jan 7, 2022
1 parent ee741de commit b5d3989
Show file tree
Hide file tree
Showing 12 changed files with 798 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,8 @@ public interface Builder {

Builder nullable(@Nullable String s);

Optional<String> nullable();

NullablePropertyWithBuilder build();
}
}
Expand All @@ -437,6 +439,9 @@ public void testOmitNullableWithBuilder() {
assertThrows(
IllegalStateException.class, () -> NullablePropertyWithBuilder.builder().build());
assertThat(e).hasMessageThat().contains("notNullable");

NullablePropertyWithBuilder.Builder builder = NullablePropertyWithBuilder.builder();
assertThat(builder.nullable()).isEmpty();
}

@AutoValue
Expand Down Expand Up @@ -496,6 +501,8 @@ public static Builder builder() {
public interface Builder {
Builder optional(@Nullable String s);

Optional<String> optional();

NullableOptionalPropertyWithNullableBuilder build();
}
}
Expand All @@ -513,6 +520,10 @@ public void testNullableOptional() {
NullableOptionalPropertyWithNullableBuilder instance3 =
NullableOptionalPropertyWithNullableBuilder.builder().optional("haruspex").build();
assertThat(instance3.optional()).hasValue("haruspex");

NullableOptionalPropertyWithNullableBuilder.Builder builder =
NullableOptionalPropertyWithNullableBuilder.builder();
assertThat(builder.optional()).isNull();
}

@AutoValue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3667,4 +3667,234 @@ public void stepBuilder() {
assertThat(stepped.two()).isEqualTo(2);
assertThat(stepped.three()).isEqualTo(3.0);
}

// The code for tracking unset properties with bitmasks is fairly tricky. When there are many
// properties, we use as many ints as needed to track which properties are still unset, with
// 32 properties being tracked per int. So here we test @AutoValue classes with 31, 32, and 33
// required properties, to catch problems at these edge values that we wouldn't see in our smaller
// test classes.
abstract static class Giant {
abstract int x1();
abstract int x2();
abstract int x3();
abstract int x4();
abstract int x5();
abstract int x6();
abstract int x7();
abstract int x8();
abstract int x9();
abstract int x10();
abstract int x11();
abstract int x12();
abstract int x13();
abstract int x14();
abstract int x15();
abstract int x16();
abstract int x17();
abstract int x18();
abstract int x19();
abstract int x20();
abstract int x21();
abstract int x22();
abstract int x23();
abstract int x24();
abstract int x25();
abstract int x26();
abstract int x27();
abstract int x28();
abstract int x29();
abstract int x30();
abstract int x31();

abstract static class Builder {
abstract Builder x1(int x);
abstract Builder x2(int x);
abstract Builder x3(int x);
abstract Builder x4(int x);
abstract Builder x5(int x);
abstract Builder x6(int x);
abstract Builder x7(int x);
abstract Builder x8(int x);
abstract Builder x9(int x);
abstract Builder x10(int x);
abstract Builder x11(int x);
abstract Builder x12(int x);
abstract Builder x13(int x);
abstract Builder x14(int x);
abstract Builder x15(int x);
abstract Builder x16(int x);
abstract Builder x17(int x);
abstract Builder x18(int x);
abstract Builder x19(int x);
abstract Builder x20(int x);
abstract Builder x21(int x);
abstract Builder x22(int x);
abstract Builder x23(int x);
abstract Builder x24(int x);
abstract Builder x25(int x);
abstract Builder x26(int x);
abstract Builder x27(int x);
abstract Builder x28(int x);
abstract Builder x29(int x);
abstract Builder x30(int x);
abstract Builder x31(int x);

Builder setFirst30() {
return this.x1(1)
.x2(2)
.x3(3)
.x4(4)
.x5(5)
.x6(6)
.x7(7)
.x8(8)
.x9(9)
.x10(10)
.x11(11)
.x12(12)
.x13(13)
.x14(14)
.x15(15)
.x16(16)
.x17(17)
.x18(18)
.x19(19)
.x20(20)
.x21(21)
.x22(22)
.x23(23)
.x24(24)
.x25(25)
.x26(26)
.x27(27)
.x28(28)
.x29(29)
.x30(30);
}
}
}

@AutoValue
abstract static class Giant31 extends Giant {
static Builder builder() {
return new AutoValue_AutoValueTest_Giant31.Builder();
}

@AutoValue.Builder
abstract static class Builder extends Giant.Builder {
abstract Giant31 build();
}
}

@AutoValue
abstract static class Giant32 extends Giant {
abstract int x32();

static Builder builder() {
return new AutoValue_AutoValueTest_Giant32.Builder();
}

@AutoValue.Builder
abstract static class Builder extends Giant.Builder {
abstract Builder x32(int x);
abstract Giant32 build();
}
}

@AutoValue
abstract static class Giant33 extends Giant {
abstract int x32();
abstract int x33();

static Builder builder() {
return new AutoValue_AutoValueTest_Giant33.Builder();
}

@AutoValue.Builder
abstract static class Builder extends Giant.Builder {
abstract Builder x32(int x);
abstract Builder x33(int x);
abstract Giant33 build();
}
}

@Test
public void testGiant31() {
Giant31.Builder builder = Giant31.builder();
builder.setFirst30();
builder.x31(31);
Giant31 giant = builder.build();
assertThat(giant.x1()).isEqualTo(1);
assertThat(giant.x31()).isEqualTo(31);

builder = Giant31.builder();
builder.setFirst30();
try {
builder.build();
fail();
} catch (IllegalStateException expected) {
if (omitIdentifiers) {
assertThat(expected).hasMessageThat().isNull();
} else {
assertThat(expected).hasMessageThat().contains("x31");
assertThat(expected).hasMessageThat().doesNotContain("x30");
}
}
}

@Test
public void testGiant32() {
Giant32.Builder builder = Giant32.builder();
builder.setFirst30();
builder.x31(31);
builder.x32(32);
Giant32 giant = builder.build();
assertThat(giant.x1()).isEqualTo(1);
assertThat(giant.x31()).isEqualTo(31);

builder = Giant32.builder();
builder.setFirst30();
try {
builder.build();
fail();
} catch (IllegalStateException expected) {
if (omitIdentifiers) {
assertThat(expected).hasMessageThat().isNull();
} else {
assertThat(expected).hasMessageThat().contains("x31");
assertThat(expected).hasMessageThat().contains("x32");
assertThat(expected).hasMessageThat().doesNotContain("x30");
}
}
}

@Test
public void testGiant33() {
Giant33.Builder builder = Giant33.builder();
builder.setFirst30();
builder.x31(31);
builder.x32(32);
builder.x33(33);
Giant33 giant = builder.build();
assertThat(giant.x1()).isEqualTo(1);
assertThat(giant.x31()).isEqualTo(31);
assertThat(giant.x32()).isEqualTo(32);
assertThat(giant.x33()).isEqualTo(33);

builder = Giant33.builder();
builder.setFirst30();
try {
builder.build();
fail();
} catch (IllegalStateException expected) {
if (omitIdentifiers) {
assertThat(expected).hasMessageThat().isNull();
} else {
assertThat(expected).hasMessageThat().contains("x31");
assertThat(expected).hasMessageThat().contains("x32");
assertThat(expected).hasMessageThat().contains("x33");
assertThat(expected).hasMessageThat().doesNotContain("x30");
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ void processType(TypeElement autoBuilderType) {
vars.builderName = TypeSimplifier.simpleNameOf(generatedClassName);
vars.builtType = TypeEncoder.encode(builtType);
vars.build = build(executable);
vars.types = typeUtils();
vars.toBuilderConstructor = false;
vars.toBuilderMethods = ImmutableList.of();
defineSharedVarsForType(autoBuilderType, ImmutableSet.of(), vars);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
import com.google.common.collect.ImmutableMultimap;
import com.google.common.collect.ImmutableSet;
import java.util.Optional;
import javax.annotation.processing.ProcessingEnvironment;
import javax.lang.model.util.Types;

/**
* Variables to substitute into the autovalue.vm or builder.vm template.
Expand Down Expand Up @@ -114,7 +112,7 @@ abstract class AutoValueOrBuilderTemplateVars extends AutoValueishTemplateVars {
* <li>it has a property-builder method (in which case it defaults to empty).
* </ul>
*/
ImmutableSet<Property> builderRequiredProperties = ImmutableSet.of();
BuilderRequiredProperties builderRequiredProperties = BuilderRequiredProperties.EMPTY;

/**
* A map from property names to information about the associated property getter. A property
Expand Down Expand Up @@ -150,7 +148,4 @@ abstract class AutoValueOrBuilderTemplateVars extends AutoValueishTemplateVars {
* subclasses)
*/
Boolean isFinal = false;

/** The type utilities returned by {@link ProcessingEnvironment#getTypeUtils()}. */
Types types;
}
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,6 @@ void processType(TypeElement type) {

String finalSubclass = TypeSimplifier.simpleNameOf(generatedSubclassName(type, 0));
AutoValueTemplateVars vars = new AutoValueTemplateVars();
vars.types = processingEnv.getTypeUtils();
vars.identifiers = !processingEnv.getOptions().containsKey(OMIT_IDENTIFIERS_OPTION);
defineSharedVarsForType(type, methods, vars);
defineVarsForType(type, vars, toBuilderMethods, propertyMethodsAndTypes, builder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public final SourceVersion getSupportedSourceVersion() {
}

/**
* A property of an {@code @AutoValue} or {@code @AutoOneOf} class, defined by one of its abstract
* A property of an {@code @AutoValue} (etc) class, defined by one of its abstract
* methods. An instance of this class is made available to the Velocity template engine for each
* property. The public methods of this class define JavaBeans-style properties that are
* accessible from templates. For example {@link #getType()} means we can write {@code $p.type}
Expand Down Expand Up @@ -231,7 +231,7 @@ public String getName() {
return name;
}

public TypeMirror getTypeMirror() {
TypeMirror getTypeMirror() {
return typeMirror;
}

Expand Down
Loading

0 comments on commit b5d3989

Please sign in to comment.