Re: Message data structure merge heads up, take 3
On Sat, Aug 14, 2010 at 3:35 AM, Emmanuel Lecharny elecha...@gmail.comwrote: One step further : - all the response have been migrated - all the ldap-api response messages have been removed and replaced by the shared-ldap response messages We now have one singe set of classes to manage responses, all over the server, except in the dsml-parser module (to be done) The next step is to add the missing toString() methods in the Message (each codec message had a toString method), then to continue the migration of requests (compare, extended, modify, modifyDn, and search). When done, we will be able to rename the Internalxxx to xxx (no reason anymore to have 'Internal' in front of each message). +1 to remove Internal designator. I'm still wondering if it's a good idea to have an interface for each message. Not sure... I favor the use of interfaces especially for API exposed elements other than helper classes. We cannot foresee all the use cases in advance and interfaces give us the greatest flexibility. A few more thing we would like to do : there are many fields which are exposed as public (the various length, used to encode the messages), and I'm wondering if we could not hide them or at least keep them package protected. To be investigated. Yeah I would imagine many things can be hidden. All in all, this cleanup has removed 24 512 slocs (from 351 355 slocs down to 326 443 slocs). Not bad ! Wow great job! Less code is always good. Thanks, -- Alex Karasulu My Blog :: http://www.jroller.com/akarasulu/ Apache Directory Server :: http://directory.apache.org Apache MINA :: http://mina.apache.org To set up a meeting with me: http://tungle.me/AlexKarasulu
Re: Message data structure merge heads up, take 3
On Sat, Aug 14, 2010 at 6:05 AM, Emmanuel Lecharny elecha...@gmail.com wrote: One step further : - all the response have been migrated - all the ldap-api response messages have been removed and replaced by the shared-ldap response messages We now have one singe set of classes to manage responses, all over the server, except in the dsml-parser module (to be done) The next step is to add the missing toString() methods in the Message (each codec message had a toString method), then to continue the migration of requests (compare, extended, modify, modifyDn, and search). When done, we will be able to rename the Internalxxx to xxx (no reason anymore to have 'Internal' in front of each message). I'm still wondering if it's a good idea to have an interface for each message. Not sure... IMO not required, i don't think this part of the server is so extendable and used anywhere else A few more thing we would like to do : there are many fields which are exposed as public (the various length, used to encode the messages), and I'm wondering if we could not hide them or at least keep them package protected. To be investigated. All in all, this cleanup has removed 24 512 slocs (from 351 355 slocs down to 326 443 slocs). Not bad ! yeah, a LOT of cleanup indeed thanks Emmanuel Kiran Ayyagari
Re: Message data structure merge heads up
On Thu, Aug 12, 2010 at 3:06 AM, Emmanuel Lécharny elecha...@apache.orgwrote: On 8/12/10 1:50 AM, Alex Karasulu wrote: On Thu, Aug 12, 2010 at 2:40 AM, Emmanuel Lecharnyelecha...@gmail.com wrote: What exactly is cleaner? Cleaner = less class, less conversion from class X to class Y. If we consider a request sent from a client using the clinet API to the server, here are the conversion we are doing atm : client - AddRequest (API class) creation --- converted to AddRequestCodec --- converted to byte[] --- ( -- network -- ) --- converted to AddRequestCodec --- converted to AddRequestImpl (internal structure used in the server --- Add processing --- AddResponseImpl creation --- conversion to AddResponseCodec --- converted to byte[] --- ( -- network -- ) --- converted to AddResponseCodec --- converted the AddResponse (Cient API class) ! Ahh OKIE you're 100% right about cleaner without this freaking mess. As we can see, there are mandatory conversions (from message to byte[] or byte[] to message), but every message --- message conversion are totaly a waste of time, CPU and code. I'm cleaning this. Excellent thanks Em. What I am afraid of is that this might be a bit more personal perspective. Nope. This has been discussed lately, and it's a decision we took in order to be able to modify the client API, as we can't transform the lookup() method to make it returning an Entry, just because of those spurious classes. Yep yep I was not completely aware of exactly what you meant. Now I'm on the same page. Thanks for taking the time to update me. God bless you for undertaking this boring and dirty task. Anyway, this is a 5 years old task whch has been postponed for ever, and august is a good period to do such modifications. Heh yeah nothing happens in Aug. Also is this being done in a branch so the net affect on the API can be evaluated? Of course ! Follow the commits ... Excellent thanks again! -- Alex Karasulu My Blog :: http://www.jroller.com/akarasulu/ Apache Directory Server :: http://directory.apache.org Apache MINA :: http://mina.apache.org To set up a meeting with me: http://tungle.me/AlexKarasulu
Re: Message data structure merge heads up
Gentlemen, I've been exploring and working my way through getting the Subversion checkout and Maven builds working reliably so that I'm comfortable with that process. I've been working on two different systems (I have a day job). My home system is Fedora Core 13 Linux with the Sun JDK 1.6_20, Subversion, Maven 2.2.1, and Eclipse 3.6 installed. On this system I had little trouble following the steps suggested: 1) Checkout trunk-with-dependencies 2) mvn clean install -DskipTests 3) mvn eclipse:eclipse I was able to import that projects into Eclipse without difficulty after the above steps. Back at a command prompt I tried running the following to make sure everything was working. 4) mvn test Only one test testSaslGssapiBind is failing, and, looking at the test code, it appears that the author doesn't expect this one to work yet. At my day job I have a Windows XP SP-3 with Sun JDK 1.6_17, Maven 2.2.1, Subversion, and Eclipse 3.6 installed. Access to the Internet from this system is restricted to using an HTTP proxy that requires NTLM authentication. I've had a bit of a struggle getting Maven and Subversion working through this but finally seem to have it (except for m2Eclipse which still seems unable to access anything through the proxy). I have noticed though that the proxy is either overloaded or just plain unreliable. I did finally succeed in getting a clean checkout, however. Again the same three steps mentioned above were completed successfully (despite the erratic operation of the proxy server). When I tried mvn test, however, I got several failures. One of the failures, testSaslGssapiBind, is the same as on Linux. For now I'm assuming this is a known problem that is being worked on. Using Eclipse to investigate each of the others I've discovered that two of them are related to Windows's use of the \ character as the path separator and one is related to incorrect handling of escaping of characters in filenames. I'm developing fixes for these issues now and will post suggested patches after I complete testing. The only remaining test failure, is testSearchUTF8 in ClientSearchRequestTest which is not throwing the expected Exception. I haven't investigated this one yet but plan to when time is available. Thanks for the guidance. Richard Feezel . This is an e-mail from General Dynamics Land Systems. It is for the intended recipient only and may contain confidential and privileged information. No one else may read, print, store, copy, forward or act in reliance on it or its attachments. If you are not the intended recipient, please return this message to the sender and delete the message and any attachments from your computer. Your cooperation is appreciated.
Re: Message data structure merge heads up
On 8/12/10 4:24 PM, feez...@gdls.com wrote: Gentlemen, Hi, My home system is Fedora Core 13 Linux with the Sun JDK 1.6_20, Subversion, Maven 2.2.1, and Eclipse 3.6 installed. Perfect. Back at a command prompt I tried running the following to make sure everything was working. 4) mvn test Only one test testSaslGssapiBind is failing, and, looking at the test code, it appears that the author doesn't expect this one to work yet. Strange. All the tests are passing on our linux machines. Have you tried mvn clean install -Dintegration ? At my day job I have a Windows XP SP-3 with Sun JDK 1.6_17, Maven 2.2.1, Subversion, and Eclipse 3.6 installed. Access to the Internet from this system is restricted to using an HTTP proxy that requires NTLM authentication. Thanks a lot, M$ ... snip/ Again the same three steps mentioned above were completed successfully (despite the erratic operation of the proxy server). When I tried mvn test, however, I got several failures. One of the failures, testSaslGssapiBind, is the same as on Linux. For now I'm assuming this is a known problem that is being worked on. Using Eclipse to investigate each of the others I've discovered that two of them are related to Windows's use of the \ character as the path separator and one is related to incorrect handling of escaping of characters in filenames. I'm developing fixes for these issues now and will post suggested patches after I complete testing. Great ! We don't use W$ at all, so it's likely we have some tests failure if we are not cautious enough. That's the price to pay for being efficient... The only remaining test failure, is testSearchUTF8 in ClientSearchRequestTest which is not throwing the expected Exception. I haven't investigated this one yet but plan to when time is available. H... I don't find this class. In which module did you found it ? -- Regards, Cordialement, Emmanuel Lécharny www.iktek.com
Re: Message data structure merge heads up, take 2
On Thu, Aug 12, 2010 at 11:41 PM, Emmanuel Lécharny elecha...@apache.org wrote: Hi guys, it's going faster than I was expecting. 40% of the messages have been merged (almost all the response though, and the resquest are a bit more complex, especially the SearchRequest) It resulted in some code shrinking : - trunk contains 351 355 slocs - branch contains 328 119 slocs This is 23 236 less slocs ! great, a lot if code shredding ;) I also expect the performance to be slightly better... for sure IMO We will see ... thanks Emmanuel Kiran Ayyagari
Re: Message data structure merge heads up
On Thu, Aug 12, 2010 at 2:40 AM, Emmanuel Lecharny elecha...@gmail.comwrote: Hi guys, just to give you some info about the ongoing merge (I'm removing the MessageCodec hierarchy, replacing it with the InternalMessage hierarchy, in the server and on the client api). So far, I have successfully been able to use the InternalMessage classes to decode the AbandonRequest, BindRequest, UnbindRequest, DelRequest. However, the associated Codec messages are still present, as the API is using them. OTOH, and because it's easier, I'm removed completely the AddResponse, BindResponse and DelResponse codec classes. They have been replaced by their equivalent classes from InternalMessage. This has been done in the clinet API and in the server. It's going fast, as all those data structure are simular. There are two things to be done when all those response messages will have been merged : - remove the API Response message classes, to use the InternalMessage response classes - Fix the dsml-parser which is using the codec hierarchy A third step would be to get rid of all the Request codec classes, this will come next. I think that I have 4 more days to get all this done. It's not exactly fun, but at least, we will have some cleaner implementation... What exactly is cleaner? What I am afraid of is that this might be a bit more personal perspective. Also is this being done in a branch so the net affect on the API can be evaluated? -- Alex Karasulu My Blog :: http://www.jroller.com/akarasulu/ Apache Directory Server :: http://directory.apache.org Apache MINA :: http://mina.apache.org To set up a meeting with me: http://tungle.me/AlexKarasulu
Re: Message data structure merge heads up
On 8/12/10 1:50 AM, Alex Karasulu wrote: On Thu, Aug 12, 2010 at 2:40 AM, Emmanuel Lecharnyelecha...@gmail.comwrote: What exactly is cleaner? Cleaner = less class, less conversion from class X to class Y. If we consider a request sent from a client using the clinet API to the server, here are the conversion we are doing atm : client - AddRequest (API class) creation --- converted to AddRequestCodec --- converted to byte[] --- ( -- network -- ) --- converted to AddRequestCodec --- converted to AddRequestImpl (internal structure used in the server --- Add processing --- AddResponseImpl creation --- conversion to AddResponseCodec --- converted to byte[] --- ( -- network -- ) --- converted to AddResponseCodec --- converted the AddResponse (Cient API class) ! As we can see, there are mandatory conversions (from message to byte[] or byte[] to message), but every message --- message conversion are totaly a waste of time, CPU and code. I'm cleaning this. What I am afraid of is that this might be a bit more personal perspective. Nope. This has been discussed lately, and it's a decision we took in order to be able to modify the client API, as we can't transform the lookup() method to make it returning an Entry, just because of those spurious classes. Anyway, this is a 5 years old task whch has been postponed for ever, and august is a good period to do such modifications. Also is this being done in a branch so the net affect on the API can be evaluated? Of course ! Follow the commits ... -- Regards, Cordialement, Emmanuel Lécharny www.iktek.com
