axis-java-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Davanum Srinivas" <dava...@gmail.com>
Subject Re: [Axis2] Issues with the current code base
Date Mon, 05 Feb 2007 03:34:12 GMT
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 <davanum@gmail.com> 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 <nagy@watson.ibm.com> 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


Mime
View raw message