cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ariel Weisberg (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-8630) Faster sequential IO (on compaction, streaming, etc)
Date Tue, 18 Aug 2015 19:42:47 GMT

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

Ariel Weisberg commented on CASSANDRA-8630:
-------------------------------------------

Doing code review now

* In RebufferingInputStream, don't throw assertion error since it's not an assert nor an Error
to read from a closed stream. JDK classes seem to throw IOException with a message. I say
don't throw anything just let it NPE since that is what the other functions do and we are
avoiding the extra branch in those. Or... check for "closed" all the time and throw IOException.
Maybe Benedict has an opinion on the performance of checking.
* RandomAccessReader.reBufferMmap() - how is mmap reading rate limited?
* RandomAccessReader.Channel is not a channel. It's more of a wrapper, descriptor, proxy or
something. 
* MemoryInputStream - This should let you read from Memories larger than 2 gigabytes right?
Ints.checkedCast in getByteBuffer will throw?
* RateLimiter is not a final class. We could start using a noop rate limiter instead of null.
* RandomAccessReader.bytesRemaining() uses Ints.checkedCast, but we expect the file to be
bigger than an int so it shouldn't throw. The API allows this and FileInputStream doesn't
throw for available. In this case saturated cast is probably the right one. We should do a
pass for Ints.checkedCast and make sure throwing is the right behavior instead of writing
handling for it.
* BufferPool.java has an import change that is extra
* I was going to ask for some warnings cleanup, but it's a big patch touching a lot of files
that already had warnings, so whatever you want to do.
* Thumbs up for logging the random seed in the tests

The approach looks good. I'm still reviewing. Working on the tests and coverage now.

> Faster sequential IO (on compaction, streaming, etc)
> ----------------------------------------------------
>
>                 Key: CASSANDRA-8630
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-8630
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Core, Tools
>            Reporter: Oleg Anastasyev
>            Assignee: Stefania
>              Labels: compaction, performance
>             Fix For: 3.x
>
>         Attachments: 8630-FasterSequencialReadsAndWrites.txt, cpu_load.png, flight_recorder_001_files.tar.gz,
flight_recorder_002_files.tar.gz, mmaped_uncomp_hotspot.png
>
>
> When node is doing a lot of sequencial IO (streaming, compacting, etc) a lot of CPU is
lost in calls to RAF's int read() and DataOutputStream's write(int).
> This is because default implementations of readShort,readLong, etc as well as their matching
write* are implemented with numerous calls of byte by byte read and write. 
> This makes a lot of syscalls as well.
> A quick microbench shows than just reimplementation of these methods in either way gives
8x speed increase.
> A patch attached implements RandomAccessReader.read<Type> and SequencialWriter.write<Type>
methods in more efficient way.
> I also eliminated some extra byte copies in CompositeType.split and ColumnNameHelper.maxComponents,
which were on my profiler's hotspot method list during tests.
> A stress tests on my laptop show that this patch makes compaction 25-30% faster  on uncompressed
sstables and 15% faster for compressed ones.
> A deployment to production shows much less CPU load for compaction. 
> (I attached a cpu load graph from one of our production, orange is niced CPU load - i.e.
compaction; yellow is user - i.e. not compaction related tasks)



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

Mime
View raw message