New ChannelBuffer type proposed
Frederic Bregier
fredbregier at free.fr
Tue Sep 1 14:51:20 EDT 2009
Just a short add on slice()
You're right, I should implement a specific slice(start, length) version
since it is legal to have readerIndex at 0 and writerIndex at length (still
keeping the extra bytes if any).
Give me a day to propose you this (it should be very easy)...
Once I've done that, I let you decide if it is good enough to replace the
current Component (which I feel a good solution since it will optimize a bit
more its already optimized current implementation) and the two methods in
ChannelBuffers for wrappedBuffer.
Cheers,
Frederic
Frederic Bregier wrote:
>
> 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-tp3496606p3562359.html
Sent from the Netty Developer Group mailing list archive at Nabble.com.
More information about the netty-dev
mailing list