[ 
https://issues.apache.org/jira/browse/FLINK-10537?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16651387#comment-16651387
 ] 

ASF GitHub Bot commented on FLINK-10537:
----------------------------------------

pnowojski commented on a change in pull request #6833: [FLINK-10537][network] 
Fix network small performance degradation after merging [FLINK-9913]
URL: https://github.com/apache/flink/pull/6833#discussion_r225459139
 
 

 ##########
 File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/api/serialization/SpanningRecordSerializer.java
 ##########
 @@ -127,6 +116,7 @@ public void reset() {
 
        @Override
        public void prune() {
+               serializationBuffer.clear();
 
 Review comment:
   Yes, I know that it will be called and that's not a bug fix. I've added this 
line when I was going though your PR trying to find any differences in 
behaviour vs the old code. I didn't expect this to be a performance issue and I 
still don't. However when I was reverting the changes I decided to keep it. 
From the consistency point of view it seems better to call clear 
(`this.position = 0;`) before/during pruning the buffer 
(`serializationBuffer.pruneBuffer();`). Otherwise for brief moment of time when 
we exit this method, we might have `position` pointing to some non existing 
place (potential `ArrayIndexOutOfBound`.
   
   Maybe `serializationBuffer .pruneBuffer` should be setting the position to 
0? Or maybe I should split this change to separate commit. As it is now, it 
suggests that it has something to do with performance (which I doubt).

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


> Network throughput performance regression after broadcast changes
> -----------------------------------------------------------------
>
>                 Key: FLINK-10537
>                 URL: https://issues.apache.org/jira/browse/FLINK-10537
>             Project: Flink
>          Issue Type: Bug
>          Components: Network
>    Affects Versions: 1.7.0
>            Reporter: Piotr Nowojski
>            Assignee: Piotr Nowojski
>            Priority: Major
>              Labels: pull-request-available
>
> There is a slight network throughput regression introduced in: 
> https://issues.apache.org/jira/browse/FLINK-9913
> It is visible in the following benchmark:
> [http://codespeed.dak8s.net:8000/timeline/#/?exe=1&ben=networkThroughput.1,100ms&env=2&revs=200&equid=off&quarts=on&extr=on]
> (drop in the chart that happened since 21st September.)



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to