[
https://issues.apache.org/jira/browse/CASSANDRA-15393?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17189237#comment-17189237
]
Marcus Eriksson commented on CASSANDRA-15393:
---------------------------------------------
First pass done, LGTM in general, I'll try to get a second pass done this week.
*Issues*
{{CounterContext.java}}
* {{hasLegacyShards(..)}} {{totalCount}} looks wrong, should probably be {{int
totalCount = (accessor.size(context) - headerLength(context, accessor)) /
STEP_LENGTH;}}
{{ByteBufferAccessor.java}}
* {{digest(..)}} needs to add {{value.position}} to {{offset}}
{{DynamicCompositeType.java}}
* in {{validateComparator(..)}}, adds {{1}} to {{offset}} after reading a
{{short}}
*Nits*
General stuff
* Slightly inconsistent use of {{TypeSizes.INT_SIZE}} /
{{TypeSize.sizeof(int)}} / hard coded {{4}}, I'd probably prefer being explicit
and use X_SIZE, mostly since we represent shorts etc in ints
* A few places added the {{left}} / {{right}} / {{accessorL}} / {{accessorR}}
parameters, but then have {{offset1/offset2/size1/size2}} - maybe rename these
to {{offsetL/offsetR}} etc
{{CQL3Type.java}}
* {{generateXCQLLiteral}}: no need to return the offset
* unused import ByteBufferUtil
{{AbstractClusteringPrefix.java}}
* maybe bring up {{size()}} and {{get(i)}} here (as {{getRawValues().length}}
and {{getRawValues()[i]}}), and only leave them overridden in
{{NativeClustering}}
{{ClusteringPrefix.java}}
* {{isBottom}} & {{isTop}} can use {{size()}} instead of
{{getRawValues().length}}
{{ArrayClusteringBound.java}} + {{ArrayClusteringBoundary.java}} +
{{BufferClusteringBound.java}} + {{BufferClusteringBoundary.java}}
* unsharedHeapSizeExcludingData unused
{{AbstractType.java}}
* {{readArray}} unused
{{ByteBufferAccessor.java}}
* in {{compare(..)}} - instead of casting to a {{ByteBuffer}} maybe use
{{accessor.toBuffer(right)}} (same in {{ByteArrayAccessor}})
{{DynamicCompositeType.java}}
* no need to {{getComparatorSize}} in {{getComparator(int i, VL left,
ValueAccessor<VL> accessorL, VR right, ValueAccessor<VR> accessorR, int
offset1, int offset2)}}
{{MapType.java}}
* don't use {{V}} as the type parameter in {{referencesUserType}} - quite
confusing as it clashes with the {{MapType<K, V>}} type param
{{ValueAccessor.java}}
* why have {{compare}} / {{compareUnsigned}} with the same parameters?
> Add byte array backed cells
> ---------------------------
>
> Key: CASSANDRA-15393
> URL: https://issues.apache.org/jira/browse/CASSANDRA-15393
> Project: Cassandra
> Issue Type: Sub-task
> Components: Local/Compaction
> Reporter: Blake Eggleston
> Assignee: Blake Eggleston
> Priority: Normal
> Fix For: 4.0-beta
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> We currently materialize all values as on heap byte buffers. Byte buffers
> have a fairly high overhead given how frequently they’re used, and on the
> compaction and local read path we don’t do anything that needs them. Use of
> byte buffer methods only happens on the coordinator. Using cells that are
> backed by byte arrays instead in these situations reduces compaction and read
> garbage up to 22% in many cases.
--
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
