David, you and I discussed about IoBuffer stuff, so let me comment about other proposed changes...
Emmanuel Lecharny wrote: > "이희승 (Trustin Lee) <[EMAIL PROTECTED]>" wrote: >> 2) Get rid of IoHandler and let IoFilter replace it. >> >> IoHandler is just a special IoFilter at the end of the filter chain. I >> don't see any reason to keep it special considering that we are often >> building multi-layered protocols. >> > I didn't went deep into this class, but if it's just the last part of a > filter, then it's just a filter like any other, a catch-all filter. So I > agree with the idea to define it as a IoFilter too. The differences between IoHandler and IoFilter are: * IoHandler doesn't have life cycle callbacks (onPreAdd, onPostRemove...) while IoFilter has. * IoHandler doesn't have handler methods for requests such as write, setTrafficMask and close while IoFilter has. This is why I'm proposing to split IoFilter into multiple interfaces: * Interface A which provides handler methods for incoming events. * Interface B which extends the interface A and provides handler methods for outgoing requests * Interface C which extends the interface B and provides lifecycle management methods This inheritance relationship might not be precise. For example, both interface B and C could extend the interface A directly. >> 3) Split IoFilter into multiple interfaces. >> >> If IoHandler is removed, IoFilter should be renamed to represent itself >> better. Moreover, IoFilter should be split into more than one interface >> (e.g. UpstreamHandler for receiving events and DownstreamHandler for >> sending events from/to an IoProcessor) so they can choose what to >> override more conveniently. >> > I'm a little bit reluctant... Filters are two-ways, so I'm not sure this > is a good idea to define at least two separate kind of filters (up and > down stream). let me explain : > > You have something similar to what Eiffel (the language) has : pre and > post processing. Calling the next filter is just an action. The flow is > much more like : > > filter N-1 -> [Filter N] pre processing... call Filter N+1... post > processing -> return to filter N-1 > > If you separate up stream and downstream filters, this will become > slightly more complicated to write, because post-processing won't be > executed, but moved to the up-stream Filter. This will split the logic > of a filter into two classes. It is true, and that's why I am proposing some inheritance relationship. Of course it's food for thought to determine which is the simpler design (i.e. w/ IoHandler vs w/o IoHandler). I personally think no IoHandler is better looking at org.apache.mina.handler.chain package. It's simply copycat of the filter chain. Yeah, I wrote that crap! :) > I have some other suggestion which is not related : The CoR we are using > is really a PITA to handle when debugging. If you don't know which > filter will be called next, either you step through a container object > before jumping into the next filter, or you set a breakpoint in every > filter (but then, if you have many filters this is a burden). > > I would rather see something really more simple, like if the call for > the next filter directly point to the next filter. Filter instanciation > will differ, but not that much. > > Atm, the logic is : > > filter N-1 > call next filter > compute next filter > call next filter > Filter N > etc... > > It would be really more simpler to have something like : > Filter N-1 > next filter.call() > Filter N > > Or at least : > Filter N-1 > next Filter = compute next filter > next filter.call() > Filter N I also want to see the stack depth as shallow as possible. This might be easier than we expect. One bad guy is the anonymous accessor methods. Removing that will decrease the stack depth quite a lot without much effort. Of course, more research will cut more out. >> 4) UDP as the first citizen transport. >> > I don't know nothing about UDP ... UDP is basically connectionless transport. MINA IoSession assumes all sessions have connection causing impedance mismatch for UDP users. That's why we have IoSessionRecycler to emulate connection. It's like using cookie to store session ID in HTTP. However, what most UDP clients and servers need is different. They just don't need such an emulation. IoHandler should provide enough information for UDP transport users; it should be provided with source and destination address of the received message, while TCP doesn't need them. The same issue lies in IoSession. IoSession.getRemoteAddress() changes whenever a new packet is received, and therefore should be considered thread-unsafe. IoSession.write(Object) has the same issue. IoSession.write(Object, SocketAddress) is actually the only valid write method in UDP. For now, emulation takes care of this issue. Then should we introduce a separate IoSession and IoHandler interfaces which is not compatible with stream-based IoSession and IoHandler interfaces? It's also food for thought; just retaining the current emulation workaround might be more consistent than introducing incompatible interfaces. Or... the UDP users might not care about this issue as long as our UDP transport performs good. :D >> Any feed back is welcome. >> > Hope that i'm not totally off rails with my feedback ! It was pretty worthwhile. As you know, discussion and realization of an idea leads us to another level of thoughts. Cheers, -- Trustin Lee - Principal Software Engineer, JBoss, Red Hat -- what we call human nature is actually human habit -- http://gleamynode.net/
signature.asc
Description: OpenPGP digital signature
