[jira] [Comment Edited] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths
[
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17100210#comment-17100210
]
Benedict Elliott Smith edited comment on CASSANDRA-15766 at 5/5/20, 7:34 PM:
-
{quote}The lambda is compiled into a static method in the byte code
{quote}
This is only true of non-capturing lambdas
edit: actually, I'm not sure what you mean here - the method _body_ will be
compiled to a static method, but the lambda will also behave like a static
_property_ if it captures no variables.
In many cases, though, even a lambda that must be allocated may be elided
entirely if the method you pass it to is inlined, and escape analysis can do
its magic - but there are a lot of 'ifs' there.
was (Author: benedict):
{quote}The lambda is compiled into a static method in the byte code
{quote}
This is only true of non-capturing lambdas
> NoSpamLogger arguments building objects on hot paths
>
>
> Key: CASSANDRA-15766
> URL: https://issues.apache.org/jira/browse/CASSANDRA-15766
> Project: Cassandra
> Issue Type: Bug
> Components: Observability/Logging
>Reporter: Jon Meredith
>Assignee: Jon Meredith
>Priority: Normal
> Fix For: 4.0-rc
>
>
> NoSpamLogger is used in hot logging paths to prevent logs being overrun. For
> that to be most effective the arguments to the logger need to be cheap to
> construct. During the internode messaging refactor CASSANDRA-15066,
> performance changes to BufferPool for CASSANDRA-14416
> were accidentally reverted in the merge up from 3.11.
> Reviewing other uses since, it looks like there are a few places where the
> arguments require some form of String building.
> org.apache.cassandra.net.InboundSink#accept
> org.apache.cassandra.net.InboundMessageHandler#processCorruptFrame
> org.apache.cassandra.net.InboundMessageHandler.LargeMessage#deserialize
> org.apache.cassandra.net.OutboundConnection#onOverloaded
> org.apache.cassandra.utils.memory.BufferPool.GlobalPool#allocateMoreChunks
> Formatting arguments should either be precomputed, or if expensive they
> should be computed after the decision on whether to noSpamLog has been made.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths
[
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17099691#comment-17099691
]
Berenguer Blasi edited comment on CASSANDRA-15766 at 5/5/20, 10:18 AM:
---
Great, Jira lost my comment. Here we go again...
I am a newbie here, so feel free to ignore me:
* I wonder why we don't defend with a {{wrapped.isLevelEnabled()}} at
{{NoSpamLogger}} top methods. It might pay off given we'd spare the many nested
method calls, the {{getStatement()}}, {{nanoTime()}}, {{shouldLog()}},... calls
* The {{Supplier}} approach is very nice, I like it a lot.
** If the overhead is negligible I would make that the only option. But given
my, maybe outdated, experiences with streams, lambdas, etc I am going to bet it
will be not :shrug:
** If it were not I'd rename top level methods as {{warnWithLazyParams()}} and
{{warnWithoutLazyParams()}}. If the dev is educated enough to have opted for
{{NoSpamLogger}} sure he will make the right choice here. I don't think we can
infer at call time which option is best, only the dev can.
+1 to a general 'other logs audit'. I would do it in another ticket as they
won't be in the hot path, they should be {{NoSpamLogger}} if they were, so that
is not as 'urgent' as this one imo.
Hope it makes sense. My 2cts
was (Author: bereng):
Great, Jira lost my comment. Here we go again...
I am a newbie here, so feel free to ignore me:
* I wonder why we don't defend with a {{wrapped.isLevelEnabled()}} at
{{NoSpamLogger}} top methods. It might pay off given we'd spare the many nested
method calls, the {{getStatement()}}, {{nanoTime()}}, {{shouldLog()},... calls
* The {{Supplier}} approach is very nice, I like it a lot.
** If the overhead is negligible I would make that the only option. But given
my, maybe outdated, experiences with streams, lambdas, etc I am going to bet it
will be not :shrug:
** If it were not I'd rename top level methods as {{warnWithLazyParams()}} and
{{warnWithoutLazyParams()}}. If the dev is educated enough to have opted for
{{NoSpamLogger}} sure he will make the right choice here. I don't think we can
infer at call time which option is best, only the dev can.
+1 to a general 'other logs audit'. I would do it in another ticket as they
won't be in the hot path, they should be {{NoSpamLogger}} if they were, so that
is not as 'urgent' as this one imo.
Hope it makes sense. My 2cts
> NoSpamLogger arguments building objects on hot paths
>
>
> Key: CASSANDRA-15766
> URL: https://issues.apache.org/jira/browse/CASSANDRA-15766
> Project: Cassandra
> Issue Type: Bug
> Components: Observability/Logging
>Reporter: Jon Meredith
>Assignee: Jon Meredith
>Priority: Normal
> Fix For: 4.0-rc
>
>
> NoSpamLogger is used in hot logging paths to prevent logs being overrun. For
> that to be most effective the arguments to the logger need to be cheap to
> construct. During the internode messaging refactor CASSANDRA-15066,
> performance changes to BufferPool for CASSANDRA-14416
> were accidentally reverted in the merge up from 3.11.
> Reviewing other uses since, it looks like there are a few places where the
> arguments require some form of String building.
> org.apache.cassandra.net.InboundSink#accept
> org.apache.cassandra.net.InboundMessageHandler#processCorruptFrame
> org.apache.cassandra.net.InboundMessageHandler.LargeMessage#deserialize
> org.apache.cassandra.net.OutboundConnection#onOverloaded
> org.apache.cassandra.utils.memory.BufferPool.GlobalPool#allocateMoreChunks
> Formatting arguments should either be precomputed, or if expensive they
> should be computed after the decision on whether to noSpamLog has been made.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths
[
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17098933#comment-17098933
]
Jon Meredith edited comment on CASSANDRA-15766 at 5/4/20, 1:36 PM:
---
I have a small prototype I was working on when I reported the issue that I
should clean up. I wanted to get some feedback on it before posting the patch,
but it probably doesn't need to wait. I'll try and push it up this morning.
The idea was too check the list of objects passed as arguments to check if they
were {{Supplier}} s and automatically call get on them if they were. The
downside would be anything that was already a supplier might need to be
double-wrapped, but I think that's worth it. I also wanted to work out what the
cost of wrapping things like
{{org.apache.cassandra.net.InboundMessageHandler#id()}} in a {{Supplier}}
versus just the call.
was (Author: jmeredithco):
I have a small prototype I was working on when I reported the issue that I
should clean up. I wanted to get some feedback on it before posting the patch,
but it probably doesn't need to wait. I'll try and push it up this morning.
The idea was too check the list of objects passed as arguments to check if they
were {{Supplier}}s and automatically call get on them if they were. The
downside would be anything that was already a supplier might need to be
double-wrapped, but I think that's worth it. I also wanted to work out what the
cost of wrapping things like
{{org.apache.cassandra.net.InboundMessageHandler#id()}} in a {{Supplier}}
versus just the call.
> NoSpamLogger arguments building objects on hot paths
>
>
> Key: CASSANDRA-15766
> URL: https://issues.apache.org/jira/browse/CASSANDRA-15766
> Project: Cassandra
> Issue Type: Bug
> Components: Observability/Logging
>Reporter: Jon Meredith
>Assignee: Jon Meredith
>Priority: Normal
> Fix For: 4.0-rc
>
>
> NoSpamLogger is used in hot logging paths to prevent logs being overrun. For
> that to be most effective the arguments to the logger need to be cheap to
> construct. During the internode messaging refactor CASSANDRA-15066,
> performance changes to BufferPool for CASSANDRA-14416
> were accidentally reverted in the merge up from 3.11.
> Reviewing other uses since, it looks like there are a few places where the
> arguments require some form of String building.
> org.apache.cassandra.net.InboundSink#accept
> org.apache.cassandra.net.InboundMessageHandler#processCorruptFrame
> org.apache.cassandra.net.InboundMessageHandler.LargeMessage#deserialize
> org.apache.cassandra.net.OutboundConnection#onOverloaded
> org.apache.cassandra.utils.memory.BufferPool.GlobalPool#allocateMoreChunks
> Formatting arguments should either be precomputed, or if expensive they
> should be computed after the decision on whether to noSpamLog has been made.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
