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 <[EMAIL PROTECTED]> 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: [EMAIL PROTECTED] > > For additional commands, e-mail: [EMAIL PROTECTED] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
-- Davanum Srinivas :: http://wso2.org/ :: Oxygen for Web Services Developers --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
