flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [flink] pnowojski commented on a change in pull request #8124: [FLINK-11877] Implement the runtime handling of the InputSelectable interface
Date Mon, 15 Apr 2019 12:57:55 GMT
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:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message