On 03/26/2011 03:49 AM, Amos Jeffries wrote: > On the cleanup branch I've been going through and trying to document > what the client-side classes do. > > As you probably know their names only have loose affiliation with their > operation and there is a LOT of scope creep blurring the boundaries. > > Below are the steps I'm seriously considering making to trunk over the > new few weeks in order to be able to clarify and document the scopes > properly. > > 1) I would like to rename ConnStateData to "ConnectionManager" since its > core scope seems to be: > * owning the client FD > * owning the request transaction state object(s) > * passing data to the request parser > * managing read/write ops with the client-streams buffers > ie (ultimate Producer for request and Consumer for reply)
I think the name should be specific to the client side unless we add a Clt namespace or some such: Clt::ConnMgr or CltConnMgr should work. You can spell those short names out more if you prefer, of course. And let's start documenting somewhere what we think those classes do now (and what we change them to do next). Is Wiki the best place for that? > 2) I would like to rename ClientSocketContext as ClientXaction since its > core scope seems to be owning the processing state flags. There is definitely a client transaction hidden among all those client-side classes. I am not sure which class should be renamed. If you think ClientSocketContext is it, and nobody has better ideas, let's try that. > This would eventually become the master transaction object, storing the > log history and master pointers to bits of data. That would be wrong, IMO. The Master Transaction class should not be tied to the client-side or any "side" transaction management. The master object must survive when the client disconnects, for example, because we may still need it for finishing adaptation, logging, etc. I would not be surprised if that MasterXact class will have virtually no "processing" code of its own, just transaction history, metadata management, and sides coordination. > Although there is a lot of change before it gets to be used everywhere. > Starting with turning its doCallouts with Async steps. One of the first steps would be to move code that belongs to one class from all the random client*cc files it appears in into the file dedicated to that class. That may be more difficult than it sounds, but it may also help you identify class boundaries and roles better. > 3) deflating the parser levels and documenting the structure of > ownership for each parse step. > I've started with breaking HttpParser out of HttpMsg.*. > > > 4) figuring out if we actually need ClientHttpRequest after the above. > > > Ideas? opinions? stuff I've overlooked? The three biggest problems with the client side that I constantly bump into is (a) it is not clear which class is responsible for what (b) sharing of client-side objects (everybody points to everybody) (c) ClientStream API (low level pointer and other's buffer manipulation) You are already working on improving (a), thank you. Anything you can do to help with (b) and (c) would be welcomed. Please do not do many things at once. If you are renaming and moving code then avoid any other changes -- it becomes impossible to review those correctly. If you are changing code in other ways, please keep your changes to the minimum so that we have a better chance of understanding the consequences while being surrounded by the client side mess. Finally, I am very concerned with your plan to commit the above changes to trunk when 3.2 is not stable yet. If the above changes are not limited to simple renaming/moving, those of us who work on the remaining 3.2 stability projects would have to switch from trunk code to the 3.2 branch. As you know from 3.0 and especially 3.1 experience, this will significantly reduce the amount of changes going back to trunk because we will be confined to our 3.2-based branches that will move away from trunk pretty fast (unlike today when we can work with trunk and trust you to carefully copy changes to the official 3.2 branch). IMO, the above changes to trunk should be postponed until we make v3.2 stable, including performance regressions, as discussed earlier. Thank you, Alex.
