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

Sylvain Lebresne commented on CASSANDRA-9797:
---------------------------------------------

bq. I mean reintroduce the write(byte[]...) implementation as we had it then, 
since it likely introduces fewer risks to restore behaviour as it was than to 
rewrite it.

That would make sense, though looking at that more closely, there has been 
enough change to {{SequentialWriter}} than just copy-pasting the old version 
doesn't at all (that old version uses {{bufferCursor()}} that doesn't exist 
anymore, it sets {{current}} even though that's not a method not a field, and 
sets {{validBufferBytes}} that also doesn't exist anymore). So we would need to 
revert a little bit more than just that one method, or modify it to fit the new 
stuffs, but both of which looks a lot more risky that the very simple version 
attached. That said, I'm also not the most familiar with the different 
evolution of {{SequentialWriter}} so if someone feels more confident with one 
of those two previous option, happy to let you have a shot at it.

> Don't wrap byte arrays in SequentialWriter
> ------------------------------------------
>
>                 Key: CASSANDRA-9797
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-9797
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Sylvain Lebresne
>            Assignee: Sylvain Lebresne
>            Priority: Minor
>              Labels: performance
>             Fix For: 3.x
>
>         Attachments: 9797.txt
>
>
> While profiling a simple stress write run ({{cassandra-stress write n=2000000 
> -rate threads=50}} to be precise) with Mission Control, I noticed that a non 
> trivial amount of heap pressure was due to the {{ByteBuffer.wrap()}} call in 
> {{SequentialWriter.write(byte[])}}. Basically, when writing a byte array, we 
> wrap it in a ByteBuffer to reuse the {{SequentialWriter.write(ByteBuffer)}} 
> method. One could have hoped this wrapping would be stack allocated, but if 
> Mission Control isn't lying (and I was told it's fairly honest on that 
> front), it's not. And we do use that {{write(byte[])}} method quite a bit, 
> especially with the new vint encodings since they use a {{byte[]}} thread 
> local buffer and call that method.
> Anyway, it sounds very simple to me to have a more direct {{write(byte[])}} 
> method, so attaching a patch to do that. A very quick local benchmark seems 
> to show a little bit less allocation and a slight edge for the branch with 
> this patch (on top of CASSANDRA-9705 I must add), but that local bench was 
> far from scientific so happy if someone that knows how to use our perf 
> service want to give that patch a shot.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to