On Fri, 31 Oct 2008 11:58:36 +0100 Emmanuel Lecharny <[EMAIL PROTECTED]> wrote:
> Julien Vermillard wrote: > > Hi, > > > Hi Julien, > > > >> (I wish I didn't forgot anything) > >> and, ultimately : > >> - It should be easy to debug > >> - it should be FAST... :) > >> > > > > And memory friendly, cause we can have 1 chain per session. > > > most certainly ! But I don't think it's a major issue :) > > <snip/> > >> 2) Possible improvements > >> ------------------------ > >> The first big point is that we currently have one single chain, > >> when having two could help a lot. The current chain is two-ways, > >> with forward links and backwards link. As we have to handle > >> incoming requests but also write outgoing response, it has to be > >> two-ways. But one single chain is certainly not the best way to > >> handle this. There is no reason why the outgoing chain should be > >> the same than the incoming chain. We may even not use the same > >> filters ! > > > > The decoder could be multi layered the the encoder not. > There is something missing in this sentence. Can you clarify ? "The decoder could be multi layered AND the encoder not." sorry for the typo :) for example you can imagine having a cumulative filter for decoding which is not needed for encoding. Or compressing one way and not one other, so you doesn't need the decompression part of the filter. > > <snip/> > >> We should not have to traverse such a high number of methods in > >> order to process the filters. Not only this is time consuming (as > >> we have to invoke as many methods as we have lines in the stack > >> trace, with all the arguments to pass), but when it comes to debug > >> filters, it's an absolute nightmare. And we have injected ONE > >> single filter in the chain ! > >> > > > > Debugging but reading the internal code too. > > > Very true ! Reading the actual code is, to be frank, a terrible > experience. Lack of documentation, lack of specification, lack of > comments, plus most certainly hacks which has been added to fulfill > some unknown requests (or may be to fix some performance issues which > have been spotted the last few years) make it almost impossible to > convince new comers to participate, due to the initial gap it costs. > This is exactly what the code should _not_ become in an Apache > project, IMHO. > > <snip/> > >> Last, not least, we are using a ChainBuilder to define the filter > >> chain. This structure is a fake FilterChain, which render the > >> manipulation tricky, and leads to confusion. I'm fine with the idea > >> of having a ChainBuilder, but it should be consistent. Another big > >> issue with this class is that it implements many useless methods > >> (like the ones which let you add or remove a filter giving its > >> class, or name, or the filter itself. One single method is > >> enough : we manipulate filters by their name... > >> > > > > I dislike this filter pick&placing API, I just teach how to use MINA > > to another guy, this API confused him, and he created the chain in > > reverse order and was unable to understand the bug. > > > Maybe we can make it more simple, and also write a better > documentation. It's really important to be able to add a new filter > live. Now, for those who build the chain in reverse order because > they get confused, I guess that if we have something which is easier > to debug, the problem will be solve quite quickly. FYI, I also have > added some toString() methods in the IoChainFilter class to expose > the current chain as a list of fiter's name, instead of a dummy list > of [EMAIL PROTECTED] (sort of :). > > <snip/> > >> So, wdyt, guys ? > >> > >> Thanks ! > >> > >> > > > > Mostly agree here, I think we need to discuss the implementation in > > detail, > Sure ! We can even split the initial mail in parts, and discuss all > of them. Even better, we can create a JIRA for each proposal, and > follow the discussion in those JIRAs. > > another painful subject is sentMessage(..) reading the IoFilter > > tutorial, explain all the braindamage it's doing. > :) I must say that, yep, you're right. I have put my feet in this > water, and it's icy... There is something badly wrong in this area, > which need to be fixed too. But I would keep it for a later > discussion, as we can't fix everything at the same time ! > > Perhaps we need to > > kill this one with this refactoring, elsewhere you will need to do > > all the "I preserve the initial message for triggering > > IoHandler.sentMessage(..)" in your new code. > > > Can you elaborate a bit ? I'm not sure that those two problems > overlap, but I may miss something, too. In IoFilter interface : public void messageSent(NextFilter nextFilter, IoSession session, Object message) which is mainly used for providing back the original message (the pojo) to the IoHandler and other filters when the final bytes of this message are wrote. I don't know exactly the implementation details but it have his root in the fitlerchain code. Julien
signature.asc
Description: PGP signature
