withOptionalProperties generates lots of garbage #160
Comments
|
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. |
|
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. |
|
Benchmarks added via #162. Ran the pertinent ones with It says:
Profiling with It says:
|
|
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;
} |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
At some point over the last few years, the implementation of
withOptionalPropertieshas gotten very wasteful. When originally implemented, the internal type was:now it is
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.
The text was updated successfully, but these errors were encountered: