Return-Path: Delivered-To: apmail-ws-axis-dev-archive@www.apache.org Received: (qmail 53305 invoked from network); 5 Feb 2007 03:34:41 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2) by minotaur.apache.org with SMTP; 5 Feb 2007 03:34:41 -0000 Received: (qmail 40083 invoked by uid 500); 5 Feb 2007 03:34:44 -0000 Delivered-To: apmail-ws-axis-dev-archive@ws.apache.org Received: (qmail 40030 invoked by uid 500); 5 Feb 2007 03:34:44 -0000 Mailing-List: contact axis-dev-help@ws.apache.org; run by ezmlm Precedence: bulk Reply-To: axis-dev@ws.apache.org list-help: list-unsubscribe: List-Post: List-Id: Delivered-To: mailing list axis-dev@ws.apache.org Received: (qmail 40007 invoked by uid 99); 5 Feb 2007 03:34:44 -0000 Received: from herse.apache.org (HELO herse.apache.org) (140.211.11.133) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 04 Feb 2007 19:34:44 -0800 X-ASF-Spam-Status: No, hits=-0.0 required=10.0 tests=SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (herse.apache.org: domain of davanum@gmail.com designates 66.249.82.228 as permitted sender) Received: from [66.249.82.228] (HELO wx-out-0506.google.com) (66.249.82.228) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 04 Feb 2007 19:34:34 -0800 Received: by wx-out-0506.google.com with SMTP id h28so1390365wxd for ; Sun, 04 Feb 2007 19:34:13 -0800 (PST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=beta; h=received:message-id:date:from:reply-to:to:subject:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references; b=XQdUg1UxdKO0rOT93omqufgOIfYfB8hA4PHzyKgq4Jd1+sczij1N/MXdopE3XhsAeP4zv6z94hyrjsGRAqKz2mktO8ywI5Fwb+/AiAP5F6gOP0PdKHlNIr49itwtZnrOVo3KoHcoVuVH60TTkShAyyew6zigcsf/H5WAVZsj3uI= Received: by 10.90.63.16 with SMTP id l16mr8167944aga.1170646453370; Sun, 04 Feb 2007 19:34:13 -0800 (PST) Received: by 10.90.106.17 with HTTP; Sun, 4 Feb 2007 19:34:12 -0800 (PST) Message-ID: <19e0530f0702041934r4e66c83cie7e4a72bde40afa4@mail.gmail.com> Date: Sun, 4 Feb 2007 22:34:12 -0500 From: "Davanum Srinivas" Reply-To: dims@apache.org To: axis-dev@ws.apache.org Subject: Re: [Axis2] Issues with the current code base In-Reply-To: <19e0530f0702041443h3e2e75e8y78e7b322f7f7ada7@mail.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <45BD9023.4050008@opensource.lk> <1170084449.10298.68.camel@scylla.watson.ibm.com> <1170094326.3447.29.camel@localhost.localdomain> <1170106620.10298.94.camel@scylla.watson.ibm.com> <19e0530f0702041443h3e2e75e8y78e7b322f7f7ada7@mail.gmail.com> X-Virus-Checked: Checked by ClamAV on apache.org Here's one more. Proliferation of getUUID's. http://ws.zones.apache.org/~dims/uuid-001.png http://ws.zones.apache.org/~dims/uuid-002.png We used to call getUUID once or at most twice. Now we create 6 uuid's for each req/resp taking up 1.9% (375/19533 * 100). Please change the code to create the correlation uuid's only when trace/debug flag is on. Otherwise, they should not be created. thanks, dims On 2/4/07, Davanum Srinivas wrote: > Bill, > > I just started looking into the perf aspects of this checkin...Here's > the first one. Please see the following 2 images: > > http://people.apache.org/~dims/msg-context-001.png > http://people.apache.org/~dims/msg-context-002.png > > As you can plainly see, the servlet is called 2500 times, and takes a > total time of 20360 ms . The method checkActivateWarning which is > basically a no-op gets called 257,500 times and takes a total of 234 > ms. Which is basically 1.14% of the total time taken to process the > messages. Am sure you agree that checkActivateWarning not present in > r495105 and was introduced later. > > So am going to get rid of it. thanks. > > thanks, > dims > > On 1/29/07, Bill Nagy wrote: > > Hi Sanjiva, > > > > On Mon, 2007-01-29 at 23:42 +0530, Sanjiva Weerawarana wrote: > > > Bill, IMO I made a mistake in lifting my -1 on this patch .. this > > > could've and should've gone in as an auxiliary bit of code without > > > modifying MC at all. Calling mc.activateMessageContext on *every* > > > message that comes in seems like a major ovehead to the mind even if not > > > to the body (you know what I mean). There was no technical need > > > whatsoever for this code to be in MC itself for Matt to be able to do > > > whatever he needs to make Sandesha persist using Java object > > > serialization instead of the serialization architecture that exists > > > now. > > > > > > Next time I won't give in so easy :). > > > > You are certainly entitled to your opinion; I did attempt to continue > > the discussion. > > > > > > > > In any case, I'm yet to see an explanation from Anne for the algorithm > > > used to figure out the parent service context etc. for a message > > > context. Anne, can you explain how you're going about figuring those > > > refs out please? Please don't say RTFS :(. > > > > > > > I will make sure that she is aware of your request. > > > > > We need to performance compare 1.1.1 with the current head and see what > > > the impact of this change is. > > > > I would be more than happy for someone to compare r495105 to r495106(the > > version where the changes were committed) and would be more than willing > > to address any performance concerns that arise from that comparison. > > > > > > > > On code conventions- search the archives please .. we've had lots of > > > discussion on this in the early days and decided on the conventions. I'm > > > pretty sure we put them in the wiki somewhere but have no idea where off > > > the top of my head :(. Interestingly, even the original JAX-WS proposal > > > by IBM mentions coding conventions: > > > http://wiki.apache.org/ws/FrontPage/Axis2/JAX-WS-Proposal. so maybe > > > someone in IBM found a ref? > > > > I was unable to find them on the wiki (I looked at both the current as > > well as the old root pages.) The JAX-WS proposal only speaks in the > > abstract about code organization -- it says nothing about the formatting > > of Java source files. Certainly you can't expect people to adhere to > > coding guidelines that only appear in email messages from almost 2 years > > ago or even know that they exist in the first place. > > > > -Bill > > > > > > > > Sanjiva. > > > > > > On Mon, 2007-01-29 at 07:27 -0800, Bill Nagy wrote: > > > > Hi Deepal, > > > > > > > > On Mon, 2007-01-29 at 11:41 +0530, Deepal Jayasinghe wrote: > > > > > Hi all; > > > > > > > > > > I just went though the current code base and realized that > > > > > MessageContext code has been changed a lot . I found few issues with the > > > > > code base and hope we need to fix them. So I thought of sending this > > > > > mail for everyone's consideration. > > > > > > > > > > Well someone please explain to me whose going to need MessageContext > > > > > serialization , > > > > > Chamikara : Do you want that for Sandesha ? > > > > > Ruchith : Do you want that for Security ? > > > > > If none of you want this , who else need this ? > > > > > > > > As Matt pointed out, we went through this on an earlier discussion -- > > > > Sandesha is the intended consumer. > > > > > > > > > > > > > I am asking this question b'coz introduction of MC serialization we have > > > > > the following for each req. > > > > > - When ever Axis2 received a message it calls > > > > > activateMessageContext(msgContext); > > > > > - And what that does is , it calls mc.activate(engineContext); method > > > > > > > > > > Unfortunately that method is TOO long and was very difficult to > > > > > understand:). Ann can you simplify the method (that will be very helpful) . > > > > > > > > Actually, if you notice, the first line of MessageContext.activate(...) > > > > is a short-circuit test on needsToBeReconciled, which is only ever set > > > > when the MessageContext (and related parts) are deserialized using the > > > > MessageContext deserialization routines -- for all other cases, it ends > > > > up being a no-op so you can stop reading at that point 8-]. As for the > > > > method being too long, of the 306 lines in that method, 110 are > > > > comments/log messages, and the rest of the code consists of > > > > if-not-null-invoke-a-method blocks. Unfortunately the MessageContext > > > > has a lot of handles to other objects, so there need to be a number of > > > > those tests. If you're having difficulty understanding a particular > > > > section of that method, please ask and we'd be happy to explain it to > > > > you. > > > > > > > > I certainly think that the rest of the code could use some "code > > > > bloat" (i.e. comments) -- e.g. there are no class-level comments on > > > > ListenerManager, AxisConfiguration, PhaseResolver (just to name a few.) > > > > > > > > > > > > > > In the meantime code convention in MC has changed a lot and need to have > > > > > very consistent code convention, please do not differ form that. > > > > > > > > This is the second time that "code conventions" have been mentioned > > > > (Sanjiva brought this up in an earlier discussion as well) -- I was not > > > > aware of these, and was unable to find anything in the docs that talk > > > > about them. (The Developer Guidelines only talk about the mailing > > > > list.) Could you please point me to where these are codified? > > > > > > > > > Among the all , the most worst thing I saw in the code is following kind > > > > > of things, I strongly believe we should not have this kind of code in > > > > > the code base, If you found such kind of code please point out them then > > > > > and there. > > > > > > > > > > - String tmpClassNameStr = "null"; (is this the way we initialize to > > > > > NULL ) > > > > > - String tmpHasList = "no list" > > > > > - Unnecessary casting > > > > > - A number of unused variables > > > > > - Variable declarations here and there (as an example private static > > > > > final String - selfManagedDataDelimiter = "*";) > > > > > > > > I'm indifferent on the first two; in some cases it makes the code easier > > > > to read and debug at the cost of an assignment and space in the string > > > > table. The third one should be caught by any decent compiler and > > > > eliminated (so long as you're not casting back and forth) and again > > > > sometimes enhances readability so I'm indifferent on this one as well. > > > > I agree on the fourth -- I don't think that there's ever a good case for > > > > extraneous variables. The fifth is again a code readability issue and > > > > one of the reasons that Java doesn't require that you declare everything > > > > up front. > > > > > > > > -Bill > > > > > > > > > > > > --------------------------------------------------------------------- > > > > To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org > > > > For additional commands, e-mail: axis-dev-help@ws.apache.org > > > > > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org > > For additional commands, e-mail: axis-dev-help@ws.apache.org > > > > > > > -- > Davanum Srinivas :: http://wso2.org/ :: Oxygen for Web Services Developers > -- Davanum Srinivas :: http://wso2.org/ :: Oxygen for Web Services Developers --------------------------------------------------------------------- To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org For additional commands, e-mail: axis-dev-help@ws.apache.org