New ChannelBuffer type proposed
Frederic Bregier
fredbregier at free.fr
Tue Sep 1 06:19:44 EDT 2009
Hi Trustin,
First, no you didn't discourage me... ;-)
In the AggregateCB, you don't have to read everything to see what I propose.
There is a
// XXX under that point: similar implementation of CompositeChannelBuffer
at line 344 that specifies that special implementation stops here.
Under that everything should be similar to CompositeChannelBuffer, except
reference to Aggregate instead of Composite and toString() which integrates
the number of slices.
What I suggest is not really to add a new type, but perhaps (if it is ok for
you) to enhance the default Composite version by get some ideas from this
Aggregate one. You've picked up some already as you state in your answer.
The main idea is to remove as many as possible slices that are already read
when it is legal to do so. Mainly it is at two stages: construction and
discard
So here are the main modifications:
In wrapped method from ChannelBuffers:
- public static ChannelBuffer wrappedCheckedBuffer(ChannelBuffer buffer)
Instead of
return buffer.slice();
doing
ChannelBuffer[] buffers = { buffer };
return new AggregateChannelBuffer(buffers);// not buffer.slice();
Since it gives a chance if the buffer is itself an Aggregate to remove
already read slices.
- public static ChannelBuffer wrappedBuffer(ChannelBuffer... buffers) {
Instead of
case 1:
if (buffers[0].readable()) {
return wrappedBuffer(buffers[0]);
}
break;
doing
case 1:
if (buffers[0].readable()) {
return new AggregateChannelBuffer(buffers);
}
break;
Again to give a chance to remove unused slices if already an aggregate one.
- public AggregateChannelBuffer(ChannelBuffer... buffers)
The constructor tryes to keep only usable part from buffers like slice()
does except that it takes care too about already aggregate buffer. In the
Composite version, Composite buffers would be just added as sub-slice, so
having slice of slice of slice... if you use the constructor several times
with the same buffer again and again like:
ChannelBuffer buffer = wrappedBuffer(buffer1, buffer2);
loop
buffer = wrappedBuffer(buffer, otherbuffer);
read some from buffer
Then buffer will be something like: (if I'm correct of course)
buffer -> slice[0]-> (slice[0]-> (slice[0] -> buffer1+slice[1] ->
buffer2)+slice[1]-> otherbuffer(1))
-> slice[1] -> otherbuffer(2)
While Aggregate will give
buffer -> slice[0] -> buffer1
-> slice[1] -> buffer2
-> slice[2] -> otherbuffer(1)
-> slice[3] -> otherbuffer(2)
and even if buffer1 and buffer2 are read when aggregation of otherbuffer(s)
comes up
buffer -> slice[0] -> otherbuffer(1)
-> slice[1] -> otherbuffer(2)
- public void writeBytes(ChannelBuffer src, int srcIndex, int length)
Again tries to add this buffer by using useful slice if it is an aggregate
one.
In fact, I'm not sure it is useful to do something specific here...
- public void discardReadBytes()
Of course, like constructor, do the real job wanted here...
In fact, for the slice(), I didn't do any specific optimization since it
produces a SlicedChannelBuffer.
Perhaps doing something close to discard could be an optimization too as you
wrote ?
So it seems you get it. Now, I probably missed something (several tries and
test to remove some bugs from time to time).
WDYT?
Cheers,
Frederic
Trustin Lee wrote:
>
> Hi Frederic,
>
> Now I've got some time to think about this new buffer type finally. :)
>
> If I understood correctly, you want to improve the performance of
> ChannelBuffer.discardReadBytes(). Am I right?
>
> If so, I think we can achieve this by either:
>
> 1) Relaxing the definition of ChannelBuffer.discardReadBytes() so that
> you can remove the reference to the discardable component instead of
> expensive memory copy:
>
> ChannelBuffer composite = ...;
> .. read some ..
> composite.discardReadBytes(); // Release the reference to the
> // discardable bytes - capacity is
> // decreased.
>
> 2) Optimizing the implementation of CompositeChannelBuffer.slice() so
> that it returns the slice that does not refer to unused components:
>
> ChannelBuffer composite = ...;
> .. read some ..
> composite = composite.slice(); // Unreadable components are
> // released.
>
> The first change needs more thoughts but the second one can be applied
> as we don't need to break backward compatibility at all.
>
> Both changes could be made as they are complimentary, and once done, we
> don't need AggregatedChannelBuffer, or am I missing out something?
>
> BTW I hope my suggestion did not discourage you. I just want to make
> sure to find the most efficient way with least effort and least change.
>
> Trustin
>
> On Sun, 23 Aug 2009 06:34:17 +0900 (KST)
> Frederic Bregier <fredbregier at free.fr> wrote:
>>
>> Hi Trustin and all,
>>
>> I've playing a bit with the Http codec and I found that having an
>> aggregate version of the channelBuffer, almost like the
>> CompositeChannelBuffer one, but that differs in such way that:
>>
>> - when an aggregation is done (at construction for instance), only
>> non empty buffer are kept.
>> - when some aggregate buffer is passed at constructor argument, they
>> are parsed from their slices too.
>>
>> The main idea is to keep the advantage of the Composite but:
>> - the memory consumption is lower by discarding unsuseful
>> channelBuffer
>> - the speed where no copy is necessary is the same than in Composite
>> - only one level of buffer most of the time is keeped (in fact 2:
>> each slice is a Slice ChannelBuffer which points to the real previous
>> ChannelBuffer)
>>
>> Therefore, it is quite efficient to do:
>>
>> ChannelBuffer mybuffer = ChannelBuffers.EMPTY_BUFFER;
>> mybuffer = AggregateChannelBuffer.wrappedCheckedBuffer(mybuffer,
>> otherbuffer, ...);
>> while (true) {
>> consumeSomeBytes(mybuffer);
>> mybuffer = AggregateChannelBuffer.wrappedCheckedBuffer(mybuffer,
>> newlyProduceBuffer);
>> }
>>
>> since there is a chance that the number of buffers in the mybuffer
>> will stay low in memory since there are consumed.
>> In the HttpDataDecoder, the FileUpload is one good example.
>>
>> Not all methods are already optimzed. For instance, it is possible to
>> optimize:
>> - writeBytes(ChannelBuffer, ...) methods
>> I try to implement in the joined implementation the method
>> writeBytes(ChannelBuffer, int, int) (the others will use it).
>>
>> Note that AggregateCB has no write capability by default (due to the
>> slice command used to add CB from constructor), like with CompositeCB
>> too (at least, if I understand well).
>> However, getting such function could help the previous code as the
>> following (simpler) but could be dizzy if the user is not warned
>> enough -he could believe that other write operations are possible- :
>>
>> ChannelBuffer mybuffer = ChannelBuffers.EMPTY_BUFFER;
>> mybuffer = AggregateChannelBuffer.wrappedCheckedBuffer(mybuffer,
>> otherbuffer, ...);
>> while (true) {
>> consumeSomeBytes(mybuffer);
>> mybuffer.writeBytes(newlyProduceBuffer);
>> }
>>
>> Perhaps better would be to create a new method addBytes(ChannelBuffer)
>> instead of writeBytes, which is more exact.
>>
>> mybuffer.addBytes(newlyProduceBuffer);
>>
>> http://n2.nabble.com/file/n3496606/AggregateChannelBuffer.java
>> AggregateChannelBuffer.java
>>
>> However, this is a proposition, perhaps I'm wrong...
>> Of course any idea or enhancement... ;-)
>>
>> Cheers,
>> Frederic
>>
>> -----
>> Hardware/Software Architect
>
>
>
> --
> Trustin Lee, http://gleamynode.net/
> _______________________________________________
> netty-dev mailing list
> netty-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/netty-dev
>
>
-----
Hardware/Software Architect
--
View this message in context: http://n2.nabble.com/New-ChannelBuffer-type-proposed-tp3496606p3559604.html
Sent from the Netty Developer Group mailing list archive at Nabble.com.
More information about the netty-dev
mailing list