cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sylvain Lebresne (JIRA)" <>
Subject [jira] [Commented] (CASSANDRA-9797) Don't wrap byte arrays in SequentialWriter
Date Fri, 17 Jul 2015 08:38:05 GMT


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:
>             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

View raw message