withOptionalProperties generates lots of garbage

[drekbour]

At some point over the last few years, the implementation of withOptionalProperties has gotten very wasteful. When originally implemented, the internal type was:

protected int value$age$int;

now it is

protected Optional<? extends Integer> value$age$int = Optional.empty();

This forces all usages to use an autoboxed Optional even if you are specifying a primitive param into a primitive field. I can see lots of work has been done in this area to support various combinations of param and field but the end result is impractical.

The (only) argument I have heard against using PB is that, in many circumstances, it represents a Poltergeist Pattern. There are many times where passing the builder around is hugely beneficial, making this a weak argument.... until now.

[Adrodoc]

I implemented it this way, because for Properties of type Optional as described in #134 the internal type had to be Optional anyways and I did not need a separate boolean field. I then used the same pattern for non-optional Properties, because I thought the code was a bit easier to understand that way. As far as I know there is no reason (besides maybe aesthetics) not to revert to Boolean- and Value-Fields for non-optional Properties when withOptionalProperties is used.

[drekbour]

Will PR some JMH benchmarks so we can be defend against performance changes before picking this one up again. Never set it this up under Gradle before so will be an interesting new challenge.

[drekbour]

Benchmarks added via #162. Ran the pertinent ones with gc profiler and get the following unsurprising results:

Benchmark                                                                                                Mode  Cnt         Score   Error   Units
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties                               thrpt            140.031           ops/s
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:·gc.alloc.rate.norm           thrpt        7203855.315            B/op
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:·gc.churn.PS_Eden_Space.norm  thrpt        7133834.507            B/op
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:·gc.count                     thrpt             83.000          counts
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:·gc.time                      thrpt             98.000              ms
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties                                   thrpt             61.656           ops/s
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:·gc.alloc.rate.norm               thrpt       18404392.246            B/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:·gc.churn.PS_Eden_Space.norm      thrpt       18400214.509            B/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:·gc.churn.PS_Survivor_Space.norm  thrpt            637.303            B/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:·gc.count                         thrpt             70.000          counts
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:·gc.time                          thrpt             90.000              ms

It says:

• neither variant generates anything permanent (alloc==churn)
• withRealProperties generates twice as much garbage

Profiling with perfnorm gives:

Benchmark                                                                                            Mode  Cnt         Score   Error  Units
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties                           thrpt            133.475          ops/s
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:L1-dcache-load-misses:u   thrpt          47802.810           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:L1-dcache-loads:u         thrpt        5095863.898           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:L1-dcache-store-misses:u  thrpt          16487.284           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:L1-dcache-stores:u        thrpt        4801208.580           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:L1-icache-load-misses:u   thrpt            534.172           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:L1-icache-loads:u         thrpt        7974251.990           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:LLC-load-misses:u         thrpt           6002.921           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:LLC-loads:u               thrpt         115690.501           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:LLC-store-misses:u        thrpt            187.161           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:LLC-stores:u              thrpt            467.724           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:branch-misses:u           thrpt            394.228           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:branches:u                thrpt        1119737.273           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:cycles:u                  thrpt        8066469.276           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:dTLB-load-misses:u        thrpt           2284.061           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:dTLB-loads:u              thrpt        5069064.855           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:dTLB-store-misses:u       thrpt             83.681           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:dTLB-stores:u             thrpt        4826016.797           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:iTLB-load-misses:u        thrpt            186.311           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:iTLB-loads:u              thrpt       14403888.321           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withOptionalProperties:instructions:u            thrpt       14515044.219           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties                               thrpt             61.299          ops/s
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:L1-dcache-load-misses:u       thrpt          70002.525           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:L1-dcache-loads:u             thrpt       12001533.316           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:L1-dcache-store-misses:u      thrpt          72868.577           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:L1-dcache-stores:u            thrpt        9265208.471           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:L1-icache-load-misses:u       thrpt           1256.561           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:L1-icache-loads:u             thrpt       17354978.628           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:LLC-load-misses:u             thrpt          25436.612           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:LLC-loads:u                   thrpt         295488.364           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:LLC-store-misses:u            thrpt            483.462           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:LLC-stores:u                  thrpt           1101.465           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:branch-misses:u               thrpt           1406.060           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:branches:u                    thrpt        2626926.750           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:cycles:u                      thrpt       17628397.648           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:dTLB-load-misses:u            thrpt           5809.171           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:dTLB-loads:u                  thrpt       12049453.672           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:dTLB-store-misses:u           thrpt           1046.858           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:dTLB-stores:u                 thrpt        9188637.498           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:iTLB-load-misses:u            thrpt            396.098           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:iTLB-loads:u                  thrpt       30576562.462           #/op
PojobuilderPerformance.constructViaOptionalBuilder_withRealProperties:instructions:u                thrpt       30475856.485           #/op

It says:

• withRealProperties required twice as many instructions
• withRealProperties required twice as much data (creating all those Optionals)
• No difference in contention (Δcycles = Δinstructions)

[drekbour]

FYI: was based on following (Java 8) style implementation whereby the stored value type is always identical to the pojo.

  public Custom4BookOptionalBuilder withTitle(String value) {
    this.value$title$java$lang$String = value;
    this.isSet$title$java$lang$String = true;
    return this;
  }

  public Custom4BookOptionalBuilder withTitle(Optional<? extends String> optionalValue) {
    optionalValue.ifPresent(this::withTitle);
    return this;
  }