As mentioned on PR, I'm not convinced by Flink's discussion and all evidence I know has shown it to have non-measurable performance impact.
I'm OK either way, though, at this point. - Whatever the consensus, let us set up checkstyle/analysis so that we maintain compatible across the codebase. - We should see what we can do to enforce not using non-serializable Optional in fields. - In special cases where you really do want an Optional stored or transmitted we could make our own SerializableOptional to still keep it compatible (example: the common Map<K, Optional<V>> representing the three states of --foo=v --foo=null and no foo specified) - Always using null & @Nullable for possible-empty fields will make the codebase poorer overall, and NPEs are still a user-facing problem that hits us pretty often. We should redouble our efforts to have a correct and fully strict analysis of these. Kenn On Wed, Aug 21, 2019 at 1:09 PM Jan Lukavský <je...@seznam.cz> wrote: > Sorry, forgot to add link to the Flink discussion [1]. > > [1] > > https://lists.apache.org/thread.html/f5f8ce92f94c9be6774340fbd7ae5e4afe07386b6765ad3cfb13aec0@%3Cdev.flink.apache.org%3E > > On 8/21/19 10:08 PM, Jan Lukavský wrote: > > Hi, > > > > sorry if this discussion have been already taken, but I'd like to know > > others opinions about how we use Optionals. The state in current > > master is as follows: > > > > $ git grep "import" | grep "java.util.Optional" | wc -l > > 85 > > $ git grep "import" | grep "Optional" | grep guava | wc -l > > 45 > > > > I'd like to propose that we use only one Optional, for consistency. > > There are arguments towards using each one of them, if I try to sum > > these up: > > > > Pros for java.util: > > > > * Part of standard lib > > > > * Will (in the future) probably better integrate with other standard > > APIs (e.g. Optional.stream in JDK 9, but probably more to come) > > > > Pros for guava: > > > > * Guava's Optional is Serializable > > > > There was recently a discussion on Flink's mailing list [1], which > > arrived at a conclusion, that using Optional as a field should be > > discouraged (in favor of @Nullable). That would imply that > > non-serializability of java.util.Optional is not an issue. But maybe > > we can arrive at a different conclusion. > > > > WDYT? > > > > Jan > > >
