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
5. if something doesn't show the results that you were expecting them,
investigate and don't ignore this! Maybe there is some larger performance
instability and your previous results were just lucky/unlucky flukes.
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
