[
https://issues.apache.org/jira/browse/FLINK-10727?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16670128#comment-16670128
]
ASF GitHub Bot commented on FLINK-10727:
----------------------------------------
pnowojski commented on a change in pull request #6974: [FLINK-10727][network]
remove unnecessary synchronization in SingleInputGate#requestPartitions()
URL: https://github.com/apache/flink/pull/6974#discussion_r229700439
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/partition/consumer/SingleInputGate.java
##########
@@ -477,8 +477,8 @@ public boolean isFinished() {
@Override
public void requestPartitions() throws IOException,
InterruptedException {
- synchronized (requestLock) {
- if (!requestedPartitionsFlag) {
+ if (!requestedPartitionsFlag) {
Review comment:
1. yes, there is this general rule that having shorter/simpler branch first:
```
if (condition) {
one_liner
}
else {
sth
very
long
that
do
not
fit
in
programmer
brain's
cache
}
```
is better, since once reader gets to the else, he can already forget about
it and focus on the else branch, since there is "all of the remaining logic".
Putting it into extreme (with `zero_liner` else branch), your example:
```
if (condition) {
sth
very
long
that
while
reading
one
is
all
the
time
unsure
what
will
happen
in
else
branch
}
```
2. you are right, it doesn't matter.
% if/else branches order, LGTM
----------------------------------------------------------------
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]
> Remove unnecessary synchronization in SingleInputGate#requestPartitions()
> -------------------------------------------------------------------------
>
> Key: FLINK-10727
> URL: https://issues.apache.org/jira/browse/FLINK-10727
> Project: Flink
> Issue Type: Improvement
> Components: Network
> Affects Versions: 1.5.5, 1.6.2, 1.7.0
> Reporter: Nico Kruber
> Assignee: Nico Kruber
> Priority: Major
> Labels: pull-request-available
>
> For every {{SingleInputGate#getNextBufferOrEvent()}},
> {{SingleInputGate#requestPartitions()}} is called and this always
> synchronizes on the {{requestLock}} before checking the
> {{requestedPartitionsFlag}}. Since {{SingleInputGate#requestPartitions()}} is
> only called from the same thread (the task thread getting the record), it is
> enough to check the {{requestedPartitionsFlag}} first before synchronizing
> for the actual requests (if needed). {{UnionInputGate}} already goes the same
> way in its {{requestPartitions()}} implementation.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
