pnowojski commented on a change in pull request #7713: [FLINK-10995][network]
Copy intermediate serialization results only once for broadcast mode
URL: https://github.com/apache/flink/pull/7713#discussion_r264314846
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/api/writer/BroadcastRecordWriter.java
##########
@@ -44,4 +52,67 @@ public BroadcastRecordWriter(
public void emit(T record) throws IOException, InterruptedException {
broadcastEmit(record);
}
+
+ @Override
+ public void broadcastEmit(T record) throws IOException,
InterruptedException {
Review comment:
Ok, I see the problem now with `randomTriggered`. I don't like it, but also
I don't have anything better to suggest :( So let it be.
> After abstracting the RecordWriter to separate two implementations, the
changes seem more complex
true. The alternate solution would be just dump the base class, but I don't
think that it would be better.
> I am not sure you could accept the way of magic 0
I noticed it before and again I'm not a big fan of it, but I can not find a
better solution.
> think my first version seems more easy to follow
I'm not sure. It had less code/changes. Issue is that in that previous
version, `BroadcastRecordWriter` was still using array of `BufferBuilder`s and
this is also confusing and probably less performant. If you could run network
benchmarks showing that both versions are equally performant I would be ok-ish
with either one.
I was expecting this to be a simple couple of lines change, but clearly I
have missed judged it :(
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
