Bill, IM 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 :).

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 :(.

We need to performance compare 1.1.1 with the current head and see what
the impact of this change is.

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:
so maybe
someone in IBM found a ref?

Sanjiva.

Mon, 2007-01-29 at 07:27 -0800, Bill Nagy wrote:
Hi Deepal,

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 T 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 (AT) ws (DOT) apache.org
For additional commands, e-mail: axis-dev-help (AT) ws (DOT) apache.org

Mon, 2007-01-29 at 13:37 -0800, Bill Nagy wrote:

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.

Absolutely; fair enough.

The basic rule was that we're following the Sun Java coding conventions
() except for the number of columns
used. I argued hard for 80 col but was voted down we settled at 100,
which was the middle between 80 and 120 ;(.

Does anyone need more details? If not please conform.

( timers: if you remember the thread and can give a ref that maybe
useful.)

If someone volunteers to put this in the Wiki in big bold letters
that'll be great.

Thanks Bill for pushing this to closure!

Sanjiva.
p.s.: Don't take this to mean that only new folks are guilty of
violating the conventions everyone is :(. But we need to try to do
better and its easier to get folks to get in line as they start
rather than later. Still, if you find offenders please jump on them! (In
a nice warm, fuzzy kinda way ;-))

FYI :

Thanks,
Ruchith

1/30/07, Sanjiva Weerawarana <sanjiva (AT) opensource (DOT) lkwrote:
Mon, 2007-01-29 at 13:37 -0800, Bill Nagy wrote:

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.

Absolutely; fair enough.

The basic rule was that we're following the Sun Java coding conventions
() except for the number of columns
used. I argued hard for 80 col but was voted down we settled at 100,
which was the middle between 80 and 120 ;(.

Does anyone need more details? If not please conform.

( timers: if you remember the thread and can give a ref that maybe
useful.)

If someone volunteers to put this in the Wiki in big bold letters
that'll be great.

Thanks Bill for pushing this to closure!

Sanjiva.
p.s.: Don't take this to mean that only new folks are guilty of
violating the conventions everyone is :(. But we need to try to do
better and its easier to get folks to get in line as they start
rather than later. Still, if you find offenders please jump on them! (In
a nice warm, fuzzy kinda way ;-))

HI all,
We already have a document which talks about the code quality [1]. We
can add whatever more details in to there We also need a BIG BLD
lettered link to this page

We have earlier agreed on using java coding conventions But IMH
It's always implied that a java developer will adhere to those. Also
coding conventions is important thing we *MUST* be looking at when we
are voting somebody as a committer. That's why we need to look at a
considerable number of commits before proposing somebody as a
committer ( course this does not apply to doc committers)

We can add these to the
document as discussed in this thread [2].

~Thilina

[1]
[2]

1/30/07, Sanjiva Weerawarana <sanjiva (AT) opensource (DOT) lkwrote:
Mon, 2007-01-29 at 13:37 -0800, Bill Nagy wrote:

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.

Absolutely; fair enough.

The basic rule was that we're following the Sun Java coding conventions
() except for the number of columns
used. I argued hard for 80 col but was voted down we settled at 100,
which was the middle between 80 and 120 ;(.

Does anyone need more details? If not please conform.

( timers: if you remember the thread and can give a ref that maybe
useful.)

If someone volunteers to put this in the Wiki in big bold letters
that'll be great.

Thanks Bill for pushing this to closure!

Sanjiva.
p.s.: Don't take this to mean that only new folks are guilty of
violating the conventions everyone is :(. But we need to try to do
better and its easier to get folks to get in line as they start
rather than later. Still, if you find offenders please jump on them! (In
a nice warm, fuzzy kinda way ;-))

Thu, 2007-02-01 at 06:10 -0800, Bill Nagy wrote:
Hi Rajith,

Usage of the serialization is/will be configurable through Sandesha --
Matt said that he would be adding code to do that (if it isn't there
already.) If the serialization doesn't get invoked, then all that will
occur will be a few no-op method invocations.

I don't agree with point 5 being a "definite no no." I believe that it
is a matter of personal preference and if the author of the code
believes that it makes it easier to read and doesn't create issues then
so be it. Any reasonable editor today will have no difficulty locating
the definition if you need to look at it.

I disagree- as an open source effort we should write in ways that
everyone can easily read and understand. These conventions are not
tailorable for personal preference!

It seems to me that most people declare variables at the top and methods
below. Is that such a hard convention to accept?

As I said, I certainly welcome any concerned party to show me (in
hard/accurate numbers) where the code does not perform and it will be
addressed.

IM its everyone's responsibility to make sure Axis2 performs well.
Telling someone else to run perf tests, do profiling and then point out
the place to fix is silly that person might as well fix it. Can't Ann
write some tests and check the perf and confirm all is koshure?

I'd prefer to be having this conversation with Ann not that I don't
like to chat with you Bill ;-)).

Sanjiva.

Thu, 2007-02-01 at 09:57 -0800, Bill Nagy wrote:

I don't honestly think that anyone who we want to be writing code for
the project would have trouble understanding code simply because braces
weren't on the lines where they typically put them or that methods
aren't named in the same way that they usually name them. With respect
to this particular issue, I don't think that the vast majority of people
go look for declarations by scrolling to the top of the file -- IDEs
have made that unnecessary. I think that we simply disagree on where to
draw the line for "reasonable" deviations from the "rules."

The whole point of having coding conventions is to get everyone to
follow them.

This is not negotiable- we have agreed on conventions and everyone
should follow them. Why is that so hard? Its good for everyone and its a
simple thing to do.

We didn't pick the conventions I liked either but it doesn't matter
that's what was decided on for the project and we must follow them.

I completely agree. The issues that have been raised so far, however,
have all been of the form "The code gets invoked on every invocation --
I don't like that." I was addressing those issues. I committed the

Those issues clearly have a performance impact!

code, therefore I am ultimately responsible for it. Before I did so, I
read through it and did my best to make sure that it was not adversely
effecting performance, and those are the points that I keep raising. Is
this a lot of code that was changed/added? Certainly. Do I believe
that it is isolated when not in use? Yes. I don't have to run
performance tests simply because somebody says so when I'm comfortable
with the logic of a particular change. Likewise, I don't believe that
anybody runs performance tests for the vast majority of the changes that
they make to the code, and they probably have thought about isolation a
lot less than was done so for this particular change.

Fair enough. Let's wait till we have some numbers if there's no
adverse impact then great.

Sanjiva.

Bill,

The same IDE that lets you find things easily, lets you reformat the
code to sun convention too and we have some additional stuff on top of
that which is also usually configurable. Definitely, there is no
excuse for not following the conventions. Yes, if you see something
wrong in someone's checkin not following the convention, you have to
let them know immediately. Please don't wait 3 months and say there
were 100 commits that did not follow the convention :)

thanks,
dims

2/2/07, Sanjiva Weerawarana <sanjiva (AT) opensource (DOT) lkwrote:
Thu, 2007-02-01 at 09:57 -0800, Bill Nagy wrote:

I don't honestly think that anyone who we want to be writing code for
the project would have trouble understanding code simply because braces
weren't on the lines where they typically put them or that methods
aren't named in the same way that they usually name them. With respect
to this particular issue, I don't think that the vast majority of people
go look for declarations by scrolling to the top of the file -- IDEs
have made that unnecessary. I think that we simply disagree on where to
draw the line for "reasonable" deviations from the "rules."

The whole point of having coding conventions is to get everyone to
follow them.

This is not negotiable- we have agreed on conventions and everyone
should follow them. Why is that so hard? Its good for everyone and its a
simple thing to do.

We didn't pick the conventions I liked either but it doesn't matter
that's what was decided on for the project and we must follow them.

I completely agree. The issues that have been raised so far, however,
have all been of the form "The code gets invoked on every invocation --
I don't like that." I was addressing those issues. I committed the

Those issues clearly have a performance impact!

code, therefore I am ultimately responsible for it. Before I did so, I
read through it and did my best to make sure that it was not adversely
effecting performance, and those are the points that I keep raising. Is
this a lot of code that was changed/added? Certainly. Do I believe
that it is isolated when not in use? Yes. I don't have to run
performance tests simply because somebody says so when I'm comfortable
with the logic of a particular change. Likewise, I don't believe that
anybody runs performance tests for the vast majority of the changes that
they make to the code, and they probably have thought about isolation a
lot less than was done so for this particular change.

Fair enough. Let's wait till we have some numbers if there's no
adverse impact then great.

Sanjiva.

+1 how about doing this just *before* cutting the release branch?
That way if the reformat causes issues it'll get fixed on the branch
(and then merged). Furthermore, when the branch is merged later it will
not appear to change everything as a reformat may.

Sanjiva.

Fri, 2007-02-02 at 10:58 -0500, Glen Daniels wrote:
I think we should also have periodic sweeps where someone uses their IDE
or another tool to do a wholesale reformat of the entire codebase to
match the conventions (this won't fix everything, no, but it can help),
and then checks in NLY the formatting changes. Maybe right before
releases?

Davanum Srinivas wrote:
Bill,

The same IDE that lets you find things easily, lets you reformat the
code to sun convention too and we have some additional stuff on top of
that which is also usually configurable. Definitely, there is no
excuse for not following the conventions. Yes, if you see something
wrong in someone's checkin not following the convention, you have to
let them know immediately. Please don't wait 3 months and say there
were 100 commits that did not follow the convention :)

thanks,
dims

2/2/07, Sanjiva Weerawarana <sanjiva (AT) opensource (DOT) lkwrote:
>Thu, 2007-02-01 at 09:57 -0800, Bill Nagy wrote:
>>
>I don't honestly think that anyone who we want to be writing code for
>the project would have trouble understanding code simply because braces
>weren't on the lines where they typically put them or that methods
>aren't named in the same way that they usually name them. With respect
>to this particular issue, I don't think that the vast majority of
>people
>go look for declarations by scrolling to the top of the file -- IDEs
>have made that unnecessary. I think that we simply disagree on
>where to
>draw the line for "reasonable" deviations from the "rules."
>>
>The whole point of having coding conventions is to get everyone to
>follow them.
>>
>This is not negotiable- we have agreed on conventions and everyone
>should follow them. Why is that so hard? Its good for everyone and its a
>simple thing to do.
>>
>We didn't pick the conventions I liked either but it doesn't matter
>that's what was decided on for the project and we must follow them.
>>
>I completely agree. The issues that have been raised so far, however,
>have all been of the form "The code gets invoked on every invocation --
>I don't like that." I was addressing those issues. I committed the
>>
>Those issues clearly have a performance impact!
>>
>code, therefore I am ultimately responsible for it. Before I did so, I
>read through it and did my best to make sure that it was not adversely
>effecting performance, and those are the points that I keep
>raising. Is
>this a lot of code that was changed/added? Certainly. Do I believe
>that it is isolated when not in use? Yes. I don't have to run
>performance tests simply because somebody says so when I'm comfortable
>with the logic of a particular change. Likewise, I don't believe that
>anybody runs performance tests for the vast majority of the changes
>that
>they make to the code, and they probably have thought about isolation a
>lot less than was done so for this particular change.
>>
>Fair enough. Let's wait till we have some numbers if there's no
>adverse impact then great.
>>
>Sanjiva.
>--
>Sanjiva Weerawarana, Ph.D.
>Founder & Director; Lanka Software Foundation; http://www.opensource.lk/
>Founder, Chairman & CE; WS, Inc.; http://www.wso2.com/
>Director; Source Initiative; http://www.opensource.org/
>Member; Apache Software Foundation; http://www.apache.org/
>Visiting Lecturer; University of Moratuwa; http://www.cse.mrt.ac.lk/
>>
>>
>
>To unsubscribe, e-mail: axis-dev-unsubscribe (AT) ws (DOT) apache.org
>For additional commands, e-mail: axis-dev-help (AT) ws (DOT) apache.org
>>
>>

To unsubscribe, e-mail: axis-dev-unsubscribe (AT) ws (DOT) apache.org
For additional commands, e-mail: axis-dev-help (AT) ws (DOT) apache.org

Bill,

I just started looking into the perf aspects of this checkinHere'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

1/29/07, Bill Nagy <nagy (AT) watson (DOT) ibm.comwrote:
Hi Sanjiva,

Mon, 2007-01-29 at 23:42 +0530, Sanjiva Weerawarana wrote:
Bill, IM 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.
--
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:
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.

Mon, 2007-01-29 at 07:27 -0800, Bill Nagy wrote:
Hi Deepal,

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 T 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 (AT) ws (DOT) apache.org
For additional commands, e-mail: axis-dev-help (AT) ws (DOT) apache.org
>
>
>

To unsubscribe, e-mail: axis-dev-unsubscribe (AT) ws (DOT) apache.org
For additional commands, e-mail: axis-dev-help (AT) ws (DOT) 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. , they should not be created.

thanks,
dims

2/4/07, Davanum Srinivas <davanum (AT) gmail (DOT) comwrote:
Bill,

I just started looking into the perf aspects of this checkinHere'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

1/29/07, Bill Nagy <nagy (AT) watson (DOT) ibm.comwrote:
Hi Sanjiva,

Mon, 2007-01-29 at 23:42 +0530, Sanjiva Weerawarana wrote:
Bill, IM 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.
--
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:
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.

Mon, 2007-01-29 at 07:27 -0800, Bill Nagy wrote:
Hi Deepal,

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 T 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 (AT) ws (DOT) apache.org
For additional commands, e-mail: axis-dev-help (AT) ws (DOT) apache.org
>
>
>

To unsubscribe, e-mail: axis-dev-unsubscribe (AT) ws (DOT) apache.org
For additional commands, e-mail: axis-dev-help (AT) ws (DOT) apache.org
>
>
>
>

Hi Dims,
Looking at uuid-001.png,
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

05/02/07, Davanum Srinivas <davanum (AT) gmail (DOT) comwrote:
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. , they should not be created.

thanks,
dims

2/4/07, Davanum Srinivas <davanum (AT) gmail (DOT) comwrote:
Bill,

I just started looking into the perf aspects of this checkinHere'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

1/29/07, Bill Nagy <nagy (AT) watson (DOT) ibm.comwrote:
Hi Sanjiva,

Mon, 2007-01-29 at 23:42 +0530, Sanjiva Weerawarana wrote:
Bill, IM 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.
--
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:
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.

Mon, 2007-01-29 at 07:27 -0800, Bill Nagy wrote:
Hi Deepal,

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 T 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 (AT) ws (DOT) apache.org
For additional commands, e-mail: axis-dev-help (AT) ws (DOT) apache.org
>
>
>

To unsubscribe, e-mail: axis-dev-unsubscribe (AT) ws (DOT) apache.org
For additional commands, e-mail: axis-dev-help (AT) ws (DOT) apache.org
>
>
>
>

David,

Latest run (fresh from svn), it's about 4%It's the getUUID's and
string concat to make the correlation id's. that's what i was talking
about :)

http://ws.zones.apache.org/~dims/msg-context-003.png
-- dims

2/5/07, David Illsley <davidillsley (AT) gmail (DOT) comwrote:
Hi Dims,
Looking at uuid-001.png,
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

05/02/07, Davanum Srinivas <davanum (AT) gmail (DOT) comwrote:
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. , they should not be created.

thanks,
dims

2/4/07, Davanum Srinivas <davanum (AT) gmail (DOT) comwrote:
Bill,

I just started looking into the perf aspects of this checkinHere'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

1/29/07, Bill Nagy <nagy (AT) watson (DOT) ibm.comwrote:
Hi Sanjiva,

Mon, 2007-01-29 at 23:42 +0530, Sanjiva Weerawarana wrote:
Bill, IM 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.
--
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:
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.

Mon, 2007-01-29 at 07:27 -0800, Bill Nagy wrote:
Hi Deepal,

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 T 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 (AT) ws (DOT) apache.org
For additional commands, e-mail: axis-dev-help (AT) ws (DOT) apache.org
>
>
>

To unsubscribe, e-mail: axis-dev-unsubscribe (AT) ws (DOT) apache.org
For additional commands, e-mail: axis-dev-help (AT) ws (DOT) apache.org
>
>
>
>

Sun, 2007-02-04 at 17:43 -0500, Davanum Srinivas wrote:
Bill,

I just started looking into the perf aspects of this checkinHere'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.

+1.

Bill/Ann, I'd like to offer to withdraw my request to make message
context de-serialization work right can you consider going back to
the original state where there was no attempt to come back correctly
from serialization? IIRC all of these extra things were added to try to
recover a correct context hierarchy and I'm not convinced it'll work
anyway. Plus its a heavy overhead.

I believe removing it does not affect your requirements for MC
serialization right? (It was put in because I complained after all.)

Sanjiva.

Sanjiva,

Hang on!. Am not done reviewing/fixing yet :) need another day or so.

thanks,
dims

2/5/07, Sanjiva Weerawarana <sanjiva (AT) opensource (DOT) lkwrote:
Sun, 2007-02-04 at 17:43 -0500, Davanum Srinivas wrote:
Bill,

I just started looking into the perf aspects of this checkinHere'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.

+1.

Bill/Ann, I'd like to offer to withdraw my request to make message
context de-serialization work right can you consider going back to
the original state where there was no attempt to come back correctly
from serialization? IIRC all of these extra things were added to try to
recover a correct context hierarchy and I'm not convinced it'll work
anyway. Plus its a heavy overhead.

I believe removing it does not affect your requirements for MC
serialization right? (It was put in because I complained after all.)

Sanjiva.

I know what you were worried about FWIW, I'm worried about the UUID
thing as well.

I'm also worried about the (now) 4% taken setting up transport
information when it's really not clear what's CPU intensive in there
(or that it should be).

David

05/02/07, Davanum Srinivas <davanum (AT) gmail (DOT) comwrote:
David,

Latest run (fresh from svn), it's about 4%It's the getUUID's and
string concat to make the correlation id's. that's what i was talking
about :)

http://ws.zones.apache.org/~dims/msg-context-003.png

-- dims

2/5/07, David Illsley <davidillsley (AT) gmail (DOT) comwrote:
Hi Dims,
Looking at uuid-001.png,
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

05/02/07, Davanum Srinivas <davanum (AT) gmail (DOT) comwrote:
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. , they should not be created.

thanks,
dims

2/4/07, Davanum Srinivas <davanum (AT) gmail (DOT) comwrote:
Bill,

I just started looking into the perf aspects of this checkinHere'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

1/29/07, Bill Nagy <nagy (AT) watson (DOT) ibm.comwrote:
Hi Sanjiva,

Mon, 2007-01-29 at 23:42 +0530, Sanjiva Weerawarana wrote:
Bill, IM 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.
--
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:
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.

Mon, 2007-01-29 at 07:27 -0800, Bill Nagy wrote:
Hi Deepal,

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 T 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 (AT) ws (DOT) apache.org
For additional commands, e-mail: axis-dev-help (AT) ws (DOT) apache.org
>
>
>

To unsubscribe, e-mail: axis-dev-unsubscribe (AT) ws (DOT) apache.org
For additional commands, e-mail: axis-dev-help (AT) ws (DOT) apache.org
>
>
>
>

Eliminated the uuid overhead (create them only when needed). We're
down to 2%. of which 1% is to get the remote address from the
servlet engine.

http://ws.zones.apache.org/~dims/msg-context-004.png

thanks,
dims

2/5/07, David Illsley <davidillsley (AT) gmail (DOT) comwrote:
I know what you were worried about FWIW, I'm worried about the UUID
thing as well.

I'm also worried about the (now) 4% taken setting up transport
information when it's really not clear what's CPU intensive in there
(or that it should be).

David

05/02/07, Davanum Srinivas <davanum (AT) gmail (DOT) comwrote:
David,

Latest run (fresh from svn), it's about 4%It's the getUUID's and
string concat to make the correlation id's. that's what i was talking
about :)

http://ws.zones.apache.org/~dims/msg-context-003.png

-- dims

2/5/07, David Illsley <davidillsley (AT) gmail (DOT) comwrote:
Hi Dims,
Looking at uuid-001.png,
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

05/02/07, Davanum Srinivas <davanum (AT) gmail (DOT) comwrote:
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. , they should not be created.

thanks,
dims

2/4/07, Davanum Srinivas <davanum (AT) gmail (DOT) comwrote:
Bill,

I just started looking into the perf aspects of this checkinHere'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

1/29/07, Bill Nagy <nagy (AT) watson (DOT) ibm.comwrote:
Hi Sanjiva,

Mon, 2007-01-29 at 23:42 +0530, Sanjiva Weerawarana wrote:
Bill, IM 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.
--
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:
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.

Mon, 2007-01-29 at 07:27 -0800, Bill Nagy wrote:
Hi Deepal,

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 T 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 (AT) ws (DOT) apache.org
For additional commands, e-mail: axis-dev-help (AT) ws (DOT) apache.org
>
>
>

To unsubscribe, e-mail: axis-dev-unsubscribe (AT) ws (DOT) apache.org
For additional commands, e-mail: axis-dev-help (AT) ws (DOT) apache.org
>
>
>
>