gortiz commented on code in PR #11205:
URL: https://github.com/apache/pinot/pull/11205#discussion_r1289886813
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/mailbox/GrpcSendingMailbox.java:
##########
@@ -61,6 +61,9 @@ public GrpcSendingMailbox(String id, ChannelManager
channelManager, String hostn
@Override
public void send(TransferableBlock block)
throws IOException {
+ if (LOGGER.isDebugEnabled()) {
+ LOGGER.debug("==[GRPC SEND]== sending data to: " + _id);
Review Comment:
There are several ways to do the logging. Usually the `{}` is the best in
terms of readability vs performance. But in terms of performance, what I'm
doing here is better. Specifically, order in performance (from higher to lower)
is:
1. add if ward and use + in the log. We don't use this option that much
because it is the worse in terms of readibility
2. use `{}`. This requires to call a `LOGGER.debug(String, Object...)`
method, so it creates an array. In case `debug` is disabled, the array is not
used, but it is very probable that the JIT will detect that and do not allocate
the array. In that case the performance of this option should be as good as 1,
but it is not guaranteed that the JIT will be smart enough. In case debug is
enabled, the array will always be created and therefore performance will be
better in 1.
4. use concatenation everywhere. This is the worse if debug is disabled
because we need to call toString in all objects and we need to do the
concatenation... just to discover that we don't actually need the String. JIT
may get rid of this, but seems very unlikely given how large the code may be
and the implications toString may have.
We can also add the if and use `{}` inside. It would probably be faster than
2, but not so much and legibility won't be great.
In this case @walterddr was worried about the performance implications of
the extra logs he added, so I tried to use the solution that give us more
performance in order to reduce that impact to the minimum
--
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.
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
