pnowojski commented on a change in pull request #8124: [FLINK-11877] Implement 
the runtime handling of the InputSelectable interface
URL: https://github.com/apache/flink/pull/8124#discussion_r275345954
 
 

 ##########
 File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/partition/consumer/UnionInputGate.java
 ##########
 @@ -197,22 +217,23 @@ public void requestPartitions() throws IOException, 
InterruptedException {
                return Optional.of(bufferOrEvent);
        }
 
-       @Override
-       public Optional<BufferOrEvent> pollNextBufferOrEvent() throws 
UnsupportedOperationException {
-               throw new UnsupportedOperationException();
-       }
-
-       private InputGateWithData waitAndGetNextInputGate() throws IOException, 
InterruptedException {
+       private InputGateWithData waitAndGetNextInputGate(boolean blocking) 
throws IOException, InterruptedException {
 
 Review comment:
   Couple of comments on a general level:
   - if you can not measure the performance difference, then just don't bother 
(avoid premature optimisations) 
   - make sure that you are not disturbing the benchmarks. While benchmarking, 
you shouldn't be touching the machine that's running the benchmarks. Scrolling 
window browser or changing windows (alt/cmd + tab) can seriously affect the 
results.
   - if in doubt, verify the results more then once, like:
   1. measure the base line
   2. measure the change, for example +5% performance improvement
   3. switch back to the base line, make sure that the result is worse
   4. go back to the change and verify +5% performance improvement
   
   I could believe that using `Optional` in `NetworkInput#pollNextElement()` 
might cause performance issues, but please double check that. I wouldn't be 
surprised that it doesn't.  `null` handling is difficult and error prone and I 
would like to stick with `Optional`'s compile time correctness checks as long 
as possible.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to