axis-java-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "David Illsley" <davidills...@gmail.com>
Subject Re: [Axis2] Issues with the current code base
Date Mon, 05 Feb 2007 08:29:12 GMT
Hi Dims,
Looking at uuid-001.png,
AxisServlet.createAndSetInitialParamsToMsgCtxt is causing 5% of the
total time which seems a lot for what I thought it was doing. I don't
have a profiler to hand, could you show us a breakdown of what that
method is calling and why it's taking so long?

Cheers,
David

On 05/02/07, Davanum Srinivas <davanum@gmail.com> wrote:
> 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
>
>


-- 
David Illsley - IBM Web Services Development

---------------------------------------------------------------------
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