[jira] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-07-22 Thread Jon Meredith (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17162882#comment-17162882
 ] 

Jon Meredith commented on CASSANDRA-15766:
--

Thanks for the quick pragmatic solution.

> 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: Caleb Rackliffe
>Priority: Normal
> Fix For: 4.0-beta2
>
>  Time Spent: 1h 40m
>  Remaining Estimate: 0h
>
> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-07-22 Thread Caleb Rackliffe (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17162876#comment-17162876
 ] 

Caleb Rackliffe commented on CASSANDRA-15766:
-

Thanks, [~aleksey] ;)

> 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: Caleb Rackliffe
>Priority: Normal
> Fix For: 4.0-beta2
>
>  Time Spent: 1h 40m
>  Remaining Estimate: 0h
>
> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-07-22 Thread Aleksey Yeschenko (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17162772#comment-17162772
 ] 

Aleksey Yeschenko commented on CASSANDRA-15766:
---

Committed to trunk as 
[1266fec349e76b964b522d11460f1df4adadcb48|https://github.com/apache/cassandra/commit/1266fec349e76b964b522d11460f1df4adadcb48].

> 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: Caleb Rackliffe
>Priority: Normal
> Fix For: 4.0-beta
>
>  Time Spent: 1h 40m
>  Remaining Estimate: 0h
>
> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-07-22 Thread Aleksey Yeschenko (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17162768#comment-17162768
 ] 

Aleksey Yeschenko commented on CASSANDRA-15766:
---

LGTM as well, will commit shortly.

> 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: Caleb Rackliffe
>Priority: Normal
> Fix For: 4.0-beta
>
>  Time Spent: 1h 40m
>  Remaining Estimate: 0h
>
> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-07-20 Thread Caleb Rackliffe (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17161532#comment-17161532
 ] 

Caleb Rackliffe commented on CASSANDRA-15766:
-

The J11 rerun is hitting CASSANDRA-15892 ({{test_resumable_rebuild}}), 
CASSANDRA-13805 ({{test_simple_parallel_repair}}), and maybe even 
CASSANDRA-15299. It's unlikely any of these is being caused by the patch. (The 
issue w/ {{SimpleReadWriteTest}} looks JVM related.)

The J8 rerun looks clean, modulo the known issues already mentioned above.

> 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: Caleb Rackliffe
>Priority: Normal
> Fix For: 4.0-beta
>
>  Time Spent: 1h 40m
>  Remaining Estimate: 0h
>
> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-07-19 Thread Caleb Rackliffe (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17160897#comment-17160897
 ] 

Caleb Rackliffe commented on CASSANDRA-15766:
-

Just a quick summary of where we are with the tests...

In the Java 11 tests, it looks like we've hit CASSANDRA-15861 ({{TestRepair:: 
test_dead_sync_initiator}}). The other 2 failures look like timeouts, something 
we could resolve with [a quick 
rerun|https://app.circleci.com/pipelines/github/maedhroz/cassandra/63/workflows/69be5c1b-f43e-4632-a146-29d546ab659d].

In the Java 8 tests, the failure in {{TestBootstrap}} has been [seen 
elsewhere|https://issues.apache.org/jira/browse/CASSANDRA-15854?focusedCommentId=17142442=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17142442],
 so is almost certainly unrelated. CASSANDRA-15861 is showing up as well. The 
interesting failure is 
{{TestDiskBalance::test_disk_balance_after_boundary_change_stcs}}, although I 
haven't been able to reproduce a failure locally. Let's see what [a 
rerun|https://app.circleci.com/pipelines/github/maedhroz/cassandra/63/workflows/99e45f4b-c7ae-4a46-8fc3-2e2466f833ed]
 looks like...

> 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: Caleb Rackliffe
>Priority: Normal
> Fix For: 4.0-beta
>
>  Time Spent: 1h 40m
>  Remaining Estimate: 0h
>
> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-07-19 Thread Caleb Rackliffe (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17160805#comment-17160805
 ] 

Caleb Rackliffe commented on CASSANDRA-15766:
-

The issue w/ {{id()}} being called for no reason in {{OutboundConnection}} was 
also identified in CASSANDRA-15700, apparently.

> 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: Caleb Rackliffe
>Priority: Normal
> Fix For: 4.0-beta
>
>  Time Spent: 1.5h
>  Remaining Estimate: 0h
>
> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-07-17 Thread Caleb Rackliffe (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17160224#comment-17160224
 ] 

Caleb Rackliffe commented on CASSANDRA-15766:
-

Did some preliminary work on this 
[here|https://github.com/maedhroz/cassandra/commit/659882f9f75eac5d019e1d935e7c18fa230f5e5a].

> 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: Caleb Rackliffe
>Priority: Normal
> Fix For: 4.0-beta
>
>
> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-07-14 Thread Berenguer Blasi (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17157460#comment-17157460
 ] 

Berenguer Blasi commented on CASSANDRA-15766:
-

Yeah that one was on my list, no worries. I'll be busy a while on a 4.0 quality 
ticket. [~maedhroz] feel free otherwise I'll steal it back from you when I 
finish lol

> 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: Caleb Rackliffe
>Priority: Normal
> Fix For: 4.0-beta
>
>
> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-07-14 Thread Jon Meredith (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17157457#comment-17157457
 ] 

Jon Meredith commented on CASSANDRA-15766:
--

It's fairly buried in my task list so if you're able to take it that would be 
great – I should have just let [~Bereng] take it - apologies.

> 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-beta
>
>
> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-07-13 Thread Caleb Rackliffe (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17157149#comment-17157149
 ] 

Caleb Rackliffe commented on CASSANDRA-15766:
-

[~jmeredithco] I'm considering taking this over after taking a quick look at 
the WIP and history behind the ticket (and dropping a [quick 
question|https://github.com/apache/cassandra/pull/582/files#r454099460] in the 
PR). For what it's worth, I probably wouldn't expand the scope beyond what's 
already been touched...to the extent I might unmark CASSANDRA-15764 as a 
duplicate.

Any objections?

> 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-beta
>
>
> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-06-18 Thread Jon Meredith (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17140037#comment-17140037
 ] 

Jon Meredith commented on CASSANDRA-15766:
--

Oh yeah, forgot that's how it went down - thanks for the reminder [~dcapwell].  
This was the Jira I filed after it was found.

> 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-beta
>
>
> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-06-18 Thread David Capwell (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17140024#comment-17140024
 ] 

David Capwell commented on CASSANDRA-15766:
---

[~jmeredithco], [~jmckenzie], [~djoshi] found a regression in networking that 
was fixed by Jeff in 3.x; checking latest trunk I don't see it fixed... 
[~djoshi] did you file? prettyPrintMemory gets called in the hot path

> 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-beta
>
>
> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-06-18 Thread Jon Meredith (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17140005#comment-17140005
 ] 

Jon Meredith commented on CASSANDRA-15766:
--

We don't currently. The original regression was spotted by code inspection 
rather than profiling, as were the other candidates mentioned in the original 
description. I think it's a fair observation that we should have a quantitive 
measure of the improvement.

I've also noticed this was tagged as 4.0-rc and moved this back to the 4.0-beta 
milestone for now. It may be possible to slip this, though the original changes 
were made due to operational issues that made the change important.

 

 

> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-06-18 Thread Josh McKenzie (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17139911#comment-17139911
 ] 

Josh McKenzie commented on CASSANDRA-15766:
---

Do we know how heavy / prevalent these calls are on a flame graph?

> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-05-05 Thread Yifan Cai (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17100214#comment-17100214
 ] 

Yifan Cai commented on CASSANDRA-15766:
---

Agree that there are many 'if's. 

What a lambda is compiled into depends on the java compiler. For a capturing 
lambda, it can still be a static method with the expanded param list. (off 
topic)

At the end, we need to run benchmark to quantitively measure the difference.

> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-05-05 Thread Benedict Elliott Smith (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17100210#comment-17100210
 ] 

Benedict Elliott Smith commented on CASSANDRA-15766:


{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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-05-05 Thread Yifan Cai (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17100159#comment-17100159
 ] 

Yifan Cai commented on CASSANDRA-15766:
---

There might not be a big difference between multiple and single lambda. The 
lambda is compiled into a static method in the bytecode view. If in the hot 
path and JIT kicked in, the static method could be even inlined. 

Consider the following code
 
{code:java}
import java.util.function.Supplier;

class Foo {
  public void foo() {
comsume(() -> "foo");
  }  
  public void comsume(Supplier supp) {
supp.get();
  }
}
{code}
Running {{javap -v -p Foo.class}} reveals the below.
{code:java}
  private static java.lang.String lambda$foo$0();
descriptor: ()Ljava/lang/String;
flags: ACC_PRIVATE, ACC_STATIC, ACC_SYNTHETIC
Code:
  stack=1, locals=0, args_size=0
 0: ldc   #5  // String foo
 2: areturn
  LineNumberTable:
line 5: 0{code}

> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-05-05 Thread Jon Meredith (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17100139#comment-17100139
 ] 

Jon Meredith commented on CASSANDRA-15766:
--

Thanks for the feedback. I was concerned it wasn't particularly idiomatic. 

My original design was to modify the actual log call to check if the statement 
should be logged, then checked if any of the format parameters were instances 
of {{Supplier and if so rebuild the parameter objects array calling 
}}{{get()}} where needed to retrieve the actual object before printing.

I backed off from that as I thought that would be multiple invocations of 
lambdas rather than a single invocation for all arguments as well as 
copying/updating the objects array (although I suppose it would be safe to 
update in place assuming it was always called as a varargs). If you're going to 
pay the cost of the lambda, maybe only worth it once. I need to play with JMH 
to understand costs better.

I'll have a go at something more idiomatic, but for cases like this it seems 
like a single lambda supplying all of the parameters would be more efficient.
{code:java}
private void onOverloaded(Message message)
{
overloadedCountUpdater.incrementAndGet(this);
int serializedSize = canonicalSize(message);
overloadedBytesUpdater.addAndGet(this, serializedSize);
noSpamLogger.warn("{} overloaded; dropping {} message (queue: {} local, {} 
endpoint, {} global)",
  lazyId,
  FBUtilities.prettyPrintMemory(serializedSize),
  FBUtilities.prettyPrintMemory(pendingBytes()),
  
FBUtilities.prettyPrintMemory(reserveCapacityInBytes.endpoint.using()),
  
FBUtilities.prettyPrintMemory(reserveCapacityInBytes.global.using()));
callbacks.onOverloaded(message, template.to);
} {code}


> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-05-05 Thread Benedict Elliott Smith (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17099731#comment-17099731
 ] 

Benedict Elliott Smith commented on CASSANDRA-15766:


Couldn't this be kept a bit more idiomatic with the provision of an object with 
a {{toString()}} method that invokes {{id()}} ? We could even have such an 
object instantiated once per {{InboundMessageHandler}} and once per 
{{BufferPool}} so that this is garbage-free for most cases, and continues to 
read like a normal logging call?

I've in the past considered introducing a special interface declaring only 
{{toString()}} for declaring these via lambdas for each parameter, which might 
be a nice way to do this for {{InboundSink}}.

A secondary API might be to provide method parameter options that accept pairs 
of (param, function) so that e.g. {{InboundSink}} can provide (t, 
{{Throwable::getMessage)}}, and if we wanted {{InboundMessageHandler}} could 
provide {{(this, InboundMessageHandler::id)}}

> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-05-05 Thread Berenguer Blasi (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17099691#comment-17099691
 ] 

Berenguer Blasi commented on CASSANDRA-15766:
-

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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-05-05 Thread Yifan Cai (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17099605#comment-17099605
 ] 

Yifan Cai commented on CASSANDRA-15766:
---

I have only done a quick glance at the patch.

Essentially, it wraps any expensive computation within the lambda, so in the 
case of the log level not permits, no CPU cycles are wasted on computing the 
log arguments that are not going to be used. The trade-off is the wrapper, 
{{Supplier}}.

Whether it is worthy or not. It depends on how expensive the unnecessary 
argument computation is. The computation itself may produce multiple 
intermediate objects. 

It would be great to have a benchmark to determine the how the lazy logging 
helps in the scenarios of 1) no computation, 2) light computation, 3) moderate 
computation and 4) heavy computation. So it serves the purpose of guidance on 
when to invoke the lazy logging and when to not. 

This patch only applies to the {{NoSpamLogger}}. There are other logging 
statements that use normal logger in the code base that may have expensive 
argument computation. If the lazy impl is proven to have performance benefits, 
how about applying to those if any? 

The patch does what is wanted in CASSANDRA-15764. I will mark CASSANDRA-15764 
as a dup and close once this one is completed. 

> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-05-04 Thread Jon Meredith (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17099431#comment-17099431
 ] 

Jon Meredith commented on CASSANDRA-15766:
--

I've pushed up my WIP, 
[branch|https://github.com/jonmeredith/cassandra/tree/15766] [and 
PR|https://github.com/apache/cassandra/pull/582]

Disassembling generated code, it looks like we trade eagerly constructing a new 
object array for an invoke dynamic call. I haven't had a chance to dig any more 
to compare the costs. I'm interested what you think.
{code:java}
  public void simpleLog();
Code:
   0: aload_0
   1: getfield  #7  // Field mock:Lorg/slf4j/Logger;
   4: getstatic #11 // Field 
org/apache/cassandra/utils/NoSpamLogger$Level.INFO:Lorg/apache/cassandra/utils/NoSpamLogger$Level;
   7: lconst_1
   8: getstatic #23 // Field 
java/util/concurrent/TimeUnit.NANOSECONDS:Ljava/util/concurrent/TimeUnit;
  11: ldc   #90 // String {}
  13: iconst_1
  14: anewarray #26 // class java/lang/Object
  17: dup
  18: iconst_0
  19: ldc   #91 // String param
  21: aastore
  22: invokestatic  #28 // Method 
org/apache/cassandra/utils/NoSpamLogger.log:(Lorg/slf4j/Logger;Lorg/apache/cassandra/utils/NoSpamLogger$Level;JLjava/util/concurrent/TimeUnit;Ljava/lang/String;[Ljava/lang/Object;)Z
  25: pop
  26: return

  public void paramLog();
Code:
   0: aload_0
   1: getfield  #7  // Field mock:Lorg/slf4j/Logger;
   4: getstatic #11 // Field 
org/apache/cassandra/utils/NoSpamLogger$Level.INFO:Lorg/apache/cassandra/utils/NoSpamLogger$Level;
   7: lconst_1
   8: getstatic #23 // Field 
java/util/concurrent/TimeUnit.NANOSECONDS:Ljava/util/concurrent/TimeUnit;
  11: ldc   #90 // String {}
  13: invokedynamic #92,  0 // InvokeDynamic 
#1:get:()Ljava/util/function/Supplier;
  18: invokestatic  #82 // Method 
org/apache/cassandra/utils/NoSpamLogger.log:(Lorg/slf4j/Logger;Lorg/apache/cassandra/utils/NoSpamLogger$Level;JLjava/util/concurrent/TimeUnit;Ljava/lang/String;Ljava/util/function/Supplier;)Z
  21: pop
  22: return
{code}

> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-05-04 Thread Berenguer Blasi (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17098960#comment-17098960
 ] 

Berenguer Blasi commented on CASSANDRA-15766:
-

+1. Either a supplier approach or a 'shouldLog()' check after getting a 
{{NoSpamLogStatement}} where the 2 options I was playing with. I'll be happy to 
help with the review. No hurries.

> 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] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-05-04 Thread Jon Meredith (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17098933#comment-17098933
 ] 

Jon Meredith commented on CASSANDRA-15766:
--

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



[jira] [Commented] (CASSANDRA-15766) NoSpamLogger arguments building objects on hot paths

2020-05-04 Thread Berenguer Blasi (Jira)


[ 
https://issues.apache.org/jira/browse/CASSANDRA-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17098818#comment-17098818
 ] 

Berenguer Blasi commented on CASSANDRA-15766:
-

Hi [~jmeredithco] I fancy taking this one. Is that ok?

> 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