[
https://issues.apache.org/jira/browse/CASSANDRA-8630?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14710183#comment-14710183
]
Ariel Weisberg edited comment on CASSANDRA-8630 at 8/24/15 10:40 PM:
---------------------------------------------------------------------
Good stuff, this is all I have at this point. I looked at coverage and things
looked good. I think it is close.
DataInputBuffer line 25, NIODataInputStream no longer has the bytes shuffling
behavior so that comment should go away.
RebufferingInputStream copy constructor appears unused (or Eclipse is lying).
It's also looks suspicious since it doesn't inherit the rebuffering behavior of
whatever it is copying?
Does ChecksummedDataInput handle files larger than 2 gigabytes? Seems like we
could end up with large hint files? The way the file based hints loop is
written it seems like it could do it. Possibly unintentionally.
The CoW idiom used for MmappedRegions seems a little off. It's making a copy on
read so every SSTableReader (they aren't shared globally I believe) will have a
separate deep copy of the entire MmappedRegions. I know this is tricky and you
probably get it better than I do, but can you get it so that the same array is
shared? Ideally both the arrays and the State object will be shared. Looking at
how the refcounting is supposed to work
The fact that MmappedRegions and it's owning MmappedSegmentedFile both are
SharedClosables seems odd to me. Seems like only one of them needs to determine
the lifetime of the whole shebang.
For rate limiting. It seems like we acquire buffer size from the rate limiter
at a time. What is the potential distribution of buffer sizes and how
reasonable are they? It seems like they can vary with the statistics of a file.
Since we got into trouble with rate limiting once I just want to be sure there
isn't a corner case where it can be a problem again.
ChecksummedDataInput test doesn't check for failing checksums. resetCrc(), and
readBytes() are also not tested.
BufferedRandomAccessFileTest.testAssertionErrorWhenBytesPastMarkIsNegative
failed for me.
CompressedRandomAccessReader.reBufferMmap() doesn't appear to be tested.
was (Author: aweisberg):
DataInputBuffer line 25, NIODataInputStream no longer has the bytes shuffling
behavior so that comment should go away.
RebufferingInputStream copy constructor appears unused (or Eclipse is lying).
It's also looks suspicious since it doesn't inherit the rebuffering behavior of
whatever it is copying?
Does ChecksummedDataInput handle files larger than 2 gigabytes? Seems like we
could end up with large hint files? The way the file based hints loop is
written it seems like it could do it. Possibly unintentionally.
The CoW idiom used for MmappedRegions seems a little off. It's making a copy on
read so every SSTableReader (they aren't shared globally I believe) will have a
separate deep copy of the entire MmappedRegions. I know this is tricky and you
probably get it better than I do, but can you get it so that the same array is
shared? Ideally both the arrays and the State object will be shared. Looking at
how the refcounting is supposed to work
The fact that MmappedRegions and it's owning MmappedSegmentedFile both are
SharedClosables seems odd to me. Seems like only one of them needs to determine
the lifetime of the whole shebang.
For rate limiting. It seems like we acquire buffer size from the rate limiter
at a time. What is the potential distribution of buffer sizes and how
reasonable are they? It seems like they can vary with the statistics of a file.
Since we got into trouble with rate limiting once I just want to be sure there
isn't a corner case where it can be a problem again.
ChecksummedDataInput test doesn't check for failing checksums. resetCrc(), and
readBytes() are also not tested.
BufferedRandomAccessFileTest.testAssertionErrorWhenBytesPastMarkIsNegative
failed for me.
CompressedRandomAccessReader.reBufferMmap() doesn't appear to be tested.
> 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)
