Hi Andrey, Thanks for working on this.
+1 it's clear and acceptable for me. To Qi, IMO the most performance critical codes are "per record" code path. We should definitely avoid Optional there. For your concern, it's "per buffer" code path which seems to be acceptable with Optional. Just one more question, is there any other code paths which are also critical? I think we'd better note that clearly. Thanks, Biao /'bɪ.aʊ/ On Fri, Aug 2, 2019 at 11:08 AM qi luo <luoqi...@gmail.com> wrote: > Agree that using Optional will improve code robustness. However we’re > hesitating to use Optional in data intensive operations. > > For example, SingleInputGate is already creating Optional for every > BufferOrEvent in getNextBufferOrEvent(). How much performance gain would we > get if it’s replaced by null check? > > Regards, > Qi > > > On Aug 1, 2019, at 11:00 PM, Andrey Zagrebin <and...@ververica.com> > wrote: > > > > Hi all, > > > > This is the next follow up discussion about suggestions for the recent > > thread about code style guide in Flink [1]. > > > > In general, one could argue that any variable, which is nullable, can be > > replaced by wrapping it with Optional to explicitly show that it can be > > null. Examples are: > > > > - returned values to force user to check not null > > - optional function arguments, e.g. with implicit default values > > - even class fields as e.g. optional config options with implicit > > default values > > > > > > At the same time, we also have @Nullable annotation to express this > > intention. > > > > Also, when the class Optional was introduced, Oracle posted a guideline > > about its usage [2]. Basically, it suggests to use it mostly in APIs for > > returned values to inform and force users to check the returned value > > instead of returning null and avoid NullPointerException. > > > > Wrapping with Optional also comes with the performance overhead. > > > > Following the Oracle's guide in general, the suggestion is: > > > > - Avoid using Optional in any performance critical code > > - Use Optional only to return nullable values in the API/public methods > > unless it is performance critical then rather use @Nullable > > - Passing an Optional argument to a method can be allowed if it is > > within a private helper method and simplifies the code, example is in > [3] > > - Optional should not be used for class fields > > > > > > Please, feel free to share you thoughts. > > > > Best, > > Andrey > > > > [1] > > > http://mail-archives.apache.org/mod_mbox/flink-dev/201906.mbox/%3ced91df4b-7cab-4547-a430-85bc710fd...@apache.org%3E > > [2] > > > https://www.oracle.com/technetwork/articles/java/java8-optional-2175753.html > > [3] > > > https://github.com/apache/flink/blob/master/flink-formats/flink-avro/src/main/java/org/apache/flink/formats/avro/typeutils/AvroFactory.java#L95 > >
