Motivation

The way that Components are added/stored in CompositeByteBuf was changed
in netty#8437 to minimize slicing and runtime access indirection, with the
obvious intention to preserve the effective behaviour of public methods.

@jingene discovered a discrepancy, reported in netty#9398, where the
component(int) and componentAtOffset(int) methods can return an
incorrect ByteBuf, i.e. not equivalent to a slice of the added buffer at
the time it was added (the previous behaviour). This can happen in
particular if the added ByteBuf is a wrapped slice, e.g. a leak aware or
unreleasable buffer.

Upon further scrutiny another subtle deviation was also noticed: When
the added `ByteBuf is a slice whose readable region does not cover the
whole buffer internalComponent(int) returns the original slice (with
capacity != readableBytes) rather than a sliced slice.

Modifications

Unfortunately to fix this robustly required slightly more invasive
change than first expected.
- A final ByteBuf srcBuf field has been added to the Component class,
which always holds the originally-added buffer. This simplifies some of
the existing fragile logic where we need access to this (e.g. when
releasing)
- Component has been made abstract with two final impls. Which of these
is used depends on whether the srcBuf is already a slice of the
component's range (or equivalent to one)
- Correctly handle the various places where access to the pre-unwrapped
buffer is important, including the problem component(int) methods
- Extend the pre-unwrapping optimization to cover WrappedByteBufs,
SwappedByteBufs, and duplicates in addition to slices
- Unit test for the original bug provided by @jingene

Result

- Correct behaviour of the (internal)component(AtOffset) methods in all
cases
- Reduced indirection when accessing components that were added from
more kinds of ByteBufs (duplicates in particular)
- Less fragile logic related to lazy-slicing components

I don't expect there to be any noticeable performance impact of
splitting Component into two subclasses since most of the methods are
still final in the superclass and the ones that aren't will benefit from
bimorphic inlining. But will aim to benchmark nonetheless.

There could be slight increase in mem use due to two additional fields
in _one_ of the two Component subtypes, but I think this is needed for
correctness and still much better than slicing components
unconditionally when adding them.


Co-authored-by: jingene <jingene0206@gmail.com>

Motivation

This is a "simpler" alternative to netty#9416 which fixes the same
CompositeByteBuf bugs described there, originally reported by @jingene
in netty#9398.

Modifications
- Add fields to Component class for the original buffer along with its
adjustment, which may be different to the already-stored unwrapped
buffer. Use it in appropriate places to ensure correctness and
equivalent behaviour to that prior to the earlier optimizations
- Add comments explaining purpose of each of the Component fields
- Unwrap more kinds of buffers in newComponent method to extend scope of
the existing indirection-reduction optimization
- De-duplicate common buffer consolidation logic
- Unit test for the original bug provided by @jingene

Result
- Correct behaviour / fixed bugs
- Some code deduplication / simplification
- Unwrapping optimization applied to more types of buffers

The downside is increased mem footprint from the two new fields, and
additional allocations in some specific cases, though those should be
rare.


Co-authored-by: jingene <jingene0206@gmail.com>

Motivation

This is a "simpler" alternative to #9416 which fixes the same
CompositeByteBuf bugs described there, originally reported by @jingene
in #9398.

Modifications
- Add fields to Component class for the original buffer along with its
adjustment, which may be different to the already-stored unwrapped
buffer. Use it in appropriate places to ensure correctness and
equivalent behaviour to that prior to the earlier optimizations
- Add comments explaining purpose of each of the Component fields
- Unwrap more kinds of buffers in newComponent method to extend scope of
the existing indirection-reduction optimization
- De-duplicate common buffer consolidation logic
- Unit test for the original bug provided by @jingene

Result
- Correct behaviour / fixed bugs
- Some code deduplication / simplification
- Unwrapping optimization applied to more types of buffers

The downside is increased mem footprint from the two new fields, and
additional allocations in some specific cases, though those should be
rare.


Co-authored-by: jingene <jingene0206@gmail.com>

Motivation

This is a "simpler" alternative to #9416 which fixes the same
CompositeByteBuf bugs described there, originally reported by @jingene
in #9398.

Modifications
- Add fields to Component class for the original buffer along with its
adjustment, which may be different to the already-stored unwrapped
buffer. Use it in appropriate places to ensure correctness and
equivalent behaviour to that prior to the earlier optimizations
- Add comments explaining purpose of each of the Component fields
- Unwrap more kinds of buffers in newComponent method to extend scope of
the existing indirection-reduction optimization
- De-duplicate common buffer consolidation logic
- Unit test for the original bug provided by @jingene

Result
- Correct behaviour / fixed bugs
- Some code deduplication / simplification
- Unwrapping optimization applied to more types of buffers

The downside is increased mem footprint from the two new fields, and
additional allocations in some specific cases, though those should be
rare.


Co-authored-by: jingene <jingene0206@gmail.com>