KAFKA-4289 - moved short-lived loggers to companion objects #2006
Conversation
|
LGTM. |
|
LGTM |
| @@ -225,7 +225,7 @@ class FileMessageSet private[kafka](@volatile var file: File, | |||
| case tl: TransportLayer => tl.transferFrom(channel, position, count) | |||
| case dc => channel.transferTo(position, count, dc) | |||
| }).toInt | |||
| trace("FileMessageSet " + file.getAbsolutePath + " : bytes transferred : " + bytesTransferred | |||
| FileMessageSet.trace("FileMessageSet " + file.getAbsolutePath + " : bytes transferred : " + bytesTransferred | |||
There was a problem hiding this comment.
You can add a import FileMessageSet._ at the top of the class block to avoid having to add the class name before trace.
| @@ -430,7 +430,7 @@ class FileMessageSet private[kafka](@volatile var file: File, | |||
|
|
|||
| } | |||
|
|
|||
| object FileMessageSet | |||
| object FileMessageSet extends Logging | |||
There was a problem hiding this comment.
This changes the logger name slightly (the class of the companion object instead of the actual class). We could override loggerName to maintain compatibility.
|
This is a good start. We should eventually evaluate logging. Scala 2.10 has macros so you can do very efficient logging calls as well. We could use https://github.com/typesafehub/scala-logging or write our own. |
|
@isolis, Kafka still supports Scala 2.10, so we can't use |
|
@ijuma I did a quick test a while ago of a way to use macros with scala. It was more of a feasibility test than a real test. Macros require a separate package so you need a 2 phase compile. Using scala-logging might be a better option. It's not urgent, obviously nobody is blocked by it, but at some point we should revisit this. Some of our profile data was showing up to 6% of the time was spent on logging, but most of it is probably on these short lived classes and they way they instantiate logging (which this patch tries to work towards). Once this is addressed the remaining part might be minor; so at some point the 0.5% performance improvement is not worth the added complexity. |
Signed-off-by: radai-rosenblatt <radai.rosenblatt@gmail.com>
radai-rosenblatt
force-pushed
the
omgsrsly
branch
from
7b475e6
to
f3127d8
Compare
|
+1 |
Signed-off-by: radai-rosenblatt radai.rosenblatt@gmail.com