mina-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From 이희승 (Trustin Lee) <trus...@gmail.com>
Subject Re: Feed back appreciated - Quick preview of buffer rewrite
Date Tue, 20 May 2008 21:36:40 GMT
Hello Daniel,

First off, thanks for the quick feed back.

On Tue, 20 May 2008 22:56:03 +0900, Daniel Wirtz <daniel@virtunity.com>  

> Hello Trustin,
>> I chose java.util.Queue because we can reuse existing interface
>> (java.util.Queue) and people knows what Queue is as they know what
>> Input/OutputStream is.
> Naming it a queue with removeXy getters instead of some non-standard
> stream sounds good so far, so the non blocking and automatic dispose  
> ideas
> become clear directly and the concept is basically the same like the
> proposed streams. However, I am still missing some setOrder(ByteOrder
> order). And what about slicing contents from the buffer? Does it *copy*  
> the
> contents to a new bytebuffer or is a composite buffer used in the  
> background
> to achive zero-copying when slicing over multiple (including partial)
> buffers? Or is it not meant to deal with a ByteBufferQueue this way and
> reuse a duplicated instance instead for further reading?

I thought about adding setOrder() method and ended up with not adding it  
because it can cause thread-safety issue when the queue is used in  
consumer-producer scenario.  For example, two threads could change the  
order at the same time and then the order can be mixed up.  Moreover, byte  
order doesn't change often, so I thought it might be a good idea to let  
user specify it when the queue is constructed.

For now, slicing might copy or might not copy depending on the specified  
position and length.  If the requested slice belongs to one ByteBuffer, no  
copy will be performed.  Otherwise a new ByteBuffer will be created and  
zero-copy in not realized.  Decoder implementor needs to understand this  
and implement the decoder properly to achieve zero-copy.  State machine  
based decoders should not have a problem with this IMHO.  Please let me  
know if I am missing something.

To provide more perfect zero-copy, we need to introduce a new type which  
wraps one or more ByteBuffers.  Its implementation could be similar to  
ByteBufferQueue, but it's different in that its length is fixed and it  
looks exactly like ByteBuffer.  It's because ByteBuffer cannot be  
extended.  Should we provide this?  I think we can.  The problem is that  
we need to write a whole bunch of JavaDoc for the new buffer class,  
although it's just one time task.

Another possible problem in introducing a new buffer type is overhead.   
ByteBufferQueue is already composite, and its elements can be composite -  
index calculation cost might neutralize the advantage of zero-copy.  On  
the other hand, we can keep the index calculation depth to two level at  
maximum and to one level in most cases, so it might not be a big problem.   

So, I'm up for introducing a new type.  However, there was also a request  
that we have to use ByteBuffer as first citizen.  Please let me know again  
if there's an issue with introducing a new type that we are missing.

>> ByteBufferQueue is what a codec or low-level filter will interact with.  
>> It
>> has all the sequential getters and putters including random access  
>> getters
>> based on byte offset.  It supports minimal set of types, but more access
>> methods will be provided as static methods of a utility class.  Our long
>> term goal is to provide a codec generater so that a user doesn't need to
>> spend a lot of time to implement any kind of codec, so keeping it  
>> minimal
>> makes sense.
> I don't really understand why it is neccessary to create an utility class
> for reading e.g. Strings instead of implementing it directly into the
> ByteBufferQueue. Or is it planned to reuse the utility class on  
> ByteBuffers
> *and* ByteBufferQueues however the getters and setters are named
> differently? Or is there a plan to provide other ByteBufferQueue
> implementations other than using...ByteBuffers? ;). If not, then I don't  
> get
> it.

Yes, we need to reuse the utility class on both ByteBuffers and  
ByteBufferQueues.  However, if we introduce a new buffer type, we can  
provide all of them to both ByteBufferQueue and the new buffer type.  To  
avoid code duplication, we will still have to keep static methods  
somewhere and make them package private.

>> All methods have been named after the methods in Queue, so it might  
>> sound a
>> little bit long, but it shouldn't be a problem in the long term with the
>> codec generator.
> For me personally this is absolutely ok, but didn't you mention yourself,
> that this will make it more complicated changing from IoBuffer upwards  
> for
> users?

I am sorry about the users.. but consistency in naming is more important  
IMHO.  We can add getters and putters which delegates all operations to  
removeXXX and offerXXX, but I guess it will make ByteBufferQueue look  
huge.  Do you have better idea?  If so, that would be fantastic.

>> It satisfies most needs except that it doesn't have additional random
>> access operations such as mark/reset, movePosition and random putters.   
>> I
>> dropped them because we can still read the buffer randomly using
>> elementAsXXX(int offset) methods.  Considering random access operations  
>> are
>> not used often, it should be OK.  Especially with the codec generator,  
>> the
>> number of random access operations should be minimal.
> I aggree. The random access getters can do the job somehow. However,  
> let's
> say an immaginary protocol uses NUL terminated strings and somewhere  
> between
> them offsets to those strings as e.g. short values and there is a need to
> randomly jump to a specific NUL terminated string inside the buffer to  
> read
> it. As far as I understood, there will be an utility class for reading
> strings. So then, will every utility method have an optional offset
> parameter? Will every random utility method return the real number of  
> bytes
> read (e.g. when reading UTF-8 Strings with multibyte characters, the real
> length cannot be determined perfectly afterwards). Without, this kind of
> protocol will be hard to decode because the utility class could not be  
> used
> at all.

Very true.  Both random access and first offset access should be provided  
for all getters and putters, and then we have no problem with such a  
protocol, right?  (just to make sure I understood the problem correctly. :)

>> PS: Yes, I think it's time to finish this stuff quickly and move on to  
>> the
>> codec generator.
> Yes, I am already curious to use the generator :).

My idea is to provide something similar to ANTLR.  The difference is it  
works in a binary level (or course including text level) and it generates  
non-blocking MINA ProtocolDecoder.  Once properly done, we could replace  
all existing codec with this and the maintenance cost will drop down  
dramatically.  Instead we will have to maintain the decoder generator big  
time, but it's more entertaining.  :)

Trustin Lee - Principal Software Engineer, JBoss, Red Hat
what we call human nature is actually human habit

View raw message