You are viewing a plain text version of this content. The canonical link for it is here.
Posted to java-dev@axis.apache.org by Glen Daniels <gl...@thoughtcraft.com> on 2007/06/25 15:46:14 UTC

Re: [Axis2] MessageContext and Options (was: Accessing properties set in options of the service client)

(note new subject line)

Hi Chinthaka:

Eran Chinthaka wrote:
> Can you remember you raised the same question during the hackathon and I
> answered it :). Anyway here it is again.

IIRC I asked about why Options were getting *copied* from place to place 
during MEPs, not why we're using the Options object as the 
MessageContext's property store... if I had realized that in Indiana, 
there would have been a longer conversation. :)

> Client should see Options object as a means to pass params to Axis2
> engine and not as a place holder for his properties. When he passes that

Right, that sounds good!

> to MessageContext, we keep that underneath the message context as we do
> not want the overhead of copying everything to a new data structure
> inside the message context. Since this options object will be anyway
> there inside the message context, and since it can store properties, we
> didn't put another object to message context to store properties. We

OK, now this really does not sound good. :)  Why does this make sense? 
I thought the idea (please correct me if I'm wrong) of Options was that 
you could reuse it across invocations, keeping all the "application 
settable" information in one place.  If that's the case, then we are 
essentially cluttering a potentially long-lived object with references 
to properties that may have no meaning outside of a given invocation.

To put it another way - if Options is actually the exact same storage 
for properties as MessageContext, it doesn't seem like there was any 
point in creating the Options object at all.  We could have just left 
the MessageContext exposed to the user and they could use it's 
setTo()/setReplyTo()/etc methods.  Or if we wanted just an API shim for 
simplicity, we could have easily provided an Options interface which 
MessageContext would implement.

> re-used the same options object. Yes, this is a deliberate design
> decision. IIRC, me and Sanjiva (Deepal also, IIRC) had a long chat on
> this, sent a proposal and did that.

I just spent a little while looking through the list archives.  I did 
see the "new client API" thread, in which Options was brought up as a 
way to capture "options to configure any MEP client".  But I did not see 
any discussion (until the code) of /replacing/ the MessageContext 
property bag with Options.  If I missed something, my apologies, but 
even if I did, I still don't agree with this design.

> See, what you are saying is in fact asking to copy the properties of the
> options object to message context property object, leaving the room for
> the user to re-use it. What we have done was asking the user to copy it
> himself, if he needs to re-use it, but achieving possible performance
> AND memory gains inside the engine.

First off, I'm not necessarily asking for any copying.  The obvious way 
to do this is to simply have the MC search the Options object if it 
doesn't find a property in its local store, just like it searches any 
other parent context.  I suspect that'll be quite sufficient, but if it 
turns out later ("don't prematurely optimize") that there's a real speed 
issue, then we can consider copying - which still shouldn't be very 
expensive (it would be odd if there were more than 50 or so things in 
there, I would guess).

As for "memory gains", I think *exactly* the opposite of what you're 
saying is true.  With the current system, any time a Handler puts an 
object into the MessageContext, there is suddenly a reference to that 
object that will hang around as long as the Options object hangs around, 
so for any kind of long-running client you've got a memory leak.  Long 
term storage is not the design center for MessageContexts!  MC's contain 
stuff that should GO AWAY after the processing of the message completes. 
  Stuff with longer lifespans should live in separate places.

> Yes, it is by design and I don't see any issue with that. It is the way
> you see it. The only problem is I am not sure we have written down this
> somewhere, other than in a mail thread :(.

If you could point me towards the mail thread that would be great.

I'd really like us to consider changing this for 1.3 and into the future.

--Glen

---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org


Re: [Axis2] MessageContext and Options

Posted by Glen Daniels <gl...@thoughtcraft.com>.
Chinthaka, all:

Check out http://svn.apache.org/viewvc?view=rev&rev=551618.

I think this is a simple change which both wins us flexibility and stops 
violating the principle of least surprise ("what are these random 
objects doing in my Options?").  Please review if you get a chance - we 
can always roll it back but I wanted folks to get a chance to see the 
very small scope of the actual changes necessary.  I'll add some more 
specific tests in a bit.

Thanks,
--Glen

---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org


Re: [Axis2] MessageContext and Options

Posted by Glen Daniels <gl...@thoughtcraft.com>.
Hi Chinthaka:

Sorry for the long message.  The "executive summary" is that I'd like to 
separate out the Options property store from the MC property store, so 
that people can reuse Options objects, avoiding the need to clone them 
and also avoiding cluttering them.  I've implemented this and it builds 
clean.  OK....

Eran Chinthaka wrote:
> IIRC, in this thread I mentioned that Options is per invocation. So if
> user wants to do multiple invocation with the same Options object then
> he needs to clone or copy it to another object.

Oh!  Is that documented somewhere?  I guess I'd expect to see something 
that important in the JavaDoc for Options, or the user guide....

So let's go with the idea that the intent of Options is NOT in fact to 
be reusable.  If that's the case, then IMHO it's still not designed very 
well.  Thoughts:

* MessageContext *is* the state-handling object associated with a given 
message.  If we designed Options purely as an API convenience (and 
really, take a look at the current API for Options - is it really 
simple?), it would have been much nicer to just supply an interface and 
leave it as one object (the MC).  What we have now is a "false" split 
into two objects, it seems to me.

* Why is MessageContext bothering to subclass AbstractContext, but not 
use the actual context-storing functionality of AC?  It's not "really" 
an AbstractContext, is it?

* We confuse what's "in" the MessageContext with what's in the Options. 
  Example - AddressingOutHandlerTest uses mc.setProperty() to set 
something, and the actual AddressingOutHandler expects to use 
Options.getProperty() to read it.  Do you think this kind of thing makes 
sense / feels natural / avoids errors?

* Options seems to really want to be associated with an Operation, not a 
Message.... it's got stuff like listeners, transport in AND out, etc. 
In fact originally it looks like it was associated with the 
OperationContext, not the MessageContext...?  See this thread:

http://marc.info/?l=axis-dev&m=113341827426019&w=2

I think things have evolved somewhat strangely....

> If you want to do multiple invocations, clone the options object. I am
> sorry I can not see a problem in that.
 > [...]
> Why do you think Options object will be there. After an invocation user
> dumps it and uses another options object.

Well, the problem with cloning it is that it's kind of a pain (which you 
mentioned in the thread referenced above :)).  Users of Axis1 are used 
to the idea that you can set up a Call, set a bunch of properties on 
that Call, and then reuse it a bunch of times, with those properties 
available in the MessageContext on each invocation.  It would be nice to 
have a similar ability in Axis2...

And in fact, it looks like a ServiceClient DOES by default re-use the 
Options object!  If I go:

ServiceClient c = new ServiceClient();
c.setOptions(opts);
c.sendReceive();
c.sendReceive();
...

it re-uses the options, no?

> So this is the summary of what I am trying to say.
> - - Options object (eventhough the name is bit weird. Dr. Sanjiva can you
> remember we discussed that the name is not appropriate to that object?)
> is per invocation only.
> - - if you want to do multiple invocations, you need to either create a
> new Options object or clone the previous one.

OK, I understand but do not agree. ;)

> In this approach, there is no un-necessary copying of properties or
> within the execution path.

According to what you're saying, you have to copy properties between 
your Options object for invocation N to a new Options object for 
invocation N+1, don't you?  That seems unnecessary to me. :)

> Glen, I apologize if I still not got the point that you are trying to
> make here. That may be why I am still thinking current impl is correct.

The point, as I hope I've clarified by now, is that the current 
implementation is flawed in one of two ways - either the Options object 
really is intended to bind to only a single MessageContext, in which 
case there are better ways to do it (i.e. provide an Options-style 
interface that MC implements, but don't have a separate object), OR the 
Options object should enable reuse of the same information across 
multiple invocations, in which case it should have a separate property 
store from the MessageContext to avoid the problems I previously 
discussed.  (Note that this technique involves no extra copies either - 
the Options object simply gets searched after the MC's normal bag for 
properties)

I've already implemented the latter, in fact - it was an easy bit of 
work, and I just ran a successful build with the changes.  So let me
toss the question back the other way... do you see anything WRONG with 
having the MC and the Options object maintain separate (but related) 
property stores?

Thanks,
--Glen

P.S.  Regardless, I think we need to review the documentation...

---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org


Re: [Axis2] MessageContext and Options

Posted by Eran Chinthaka <ch...@opensource.lk>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

IIRC, in this thread I mentioned that Options is per invocation. So if
user wants to do multiple invocation with the same Options object then
he needs to clone or copy it to another object.

So as you have illustrated, you CAN NOT use the same options within that
that loop. What we can do is provide a clone method and let the user
copy it to a new Options object.

Ok, there are two cases.

1. User creates and Options object and do an invocation. In that case we
do not need to copy all the properties in to the message context as we
are using the same Options object.

2. User creates loops, access the same method several times, same
handler deployed in several places and tries to use the same Options.

IMO, first case is very much used than the second case. I prefer to give
priority to the base case than to the complex case, if the complex case
also works with that method. So if the same Options object is used
within the engine also then it is simpler even within the engine code.


Glen Daniels wrote:
> 
> A little code:
> 
> public class MyTestProgram {
>   public static void main() throws Exception {
>     Options opts = new Options();
>     // ...initialize options...
>     while (a_very_long_time) {
>       ServiceClient client = new ServiceClient(opts);
>       // ... do some more stuff ...
>       client.sendReceive(...);
>     }
>   }
> }
> 
> So every time around that loop, that Options object is becoming not only
> the storage for the stuff that the user put there, but also for any
> properties that the Handlers which run happen to put there.  Let's say
> you've got a Handler that can be deployed multiple times on a given
> flow; one that performs some function and then counts the number of
> times it has been invoked in a given message flow.  So it has code like:
> 
>   Integer count = (Integer)msgContext.getProperty("myCount");
>   if (count == null) count = new Integer(0);
>   count = new Integer(count.intValue() + 1);
>   msgContext.setProperty("myCount");
> 
> Now, the idea is to record the number of invocations per MESSAGE, which
> is why it's writing to the *Message*Context.  You can see how the second
> time around the loop, the count becomes incorrect, right?  Because the
> property isn't per-MessageContext, it's per-Options.  Bad.

well, this happens only if the user uses the *same* Options object for
multiple invocations, which is wrong. That is why I mentioned in one of
my earlier emails also that Options object is per invocation.

If you want to do multiple invocations, clone the options object. I am
sorry I can not see a problem in that.

> 
> That's one kind of problem.  Another is that Handlers might use largish
> data structures and put them in MessageContext properties - any such
> structures won't get GC'ed for as long as the Options object is held...
> and that might be a long time (for instance I can imagine an Axis2
> system running in a continuous loop polling shop floor equipment
> checking for alert conditions - too hot, low on oil, etc).

Why do you think Options object will be there. After an invocation user
dumps it and uses another options object.

So this is the summary of what I am trying to say.
- - Options object (eventhough the name is bit weird. Dr. Sanjiva can you
remember we discussed that the name is not appropriate to that object?)
is per invocation only.
- - if you want to do multiple invocations, you need to either create a
new Options object or clone the previous one.

In this approach, there is no un-necessary copying of properties or
within the execution path.

Glen, I apologize if I still not got the point that you are trying to
make here. That may be why I am still thinking current impl is correct.

Chinthaka
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGgdLtjON2uBzUhh8RArZRAJ4pGt2VWMRGwHekjN/ATNZ246VIagCeIFU/
+LHUDco0L/KNlWt3E+K/oSI=
=KG3D
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org


Re: [Axis2] MessageContext and Options

Posted by Glen Daniels <gl...@thoughtcraft.com>.
Hi Chinthaka:

Eran Chinthaka wrote:
> Well, I don't think exposing message context is an option as it has lots
> of methods and complex. Yes we could come up with an alternative to

I don't think it is (or perhaps "should be") all that complex, really.

> implement options interface to message context, but we didn't consider
> that alternative when we were implementing it. The current method was
> the best one that we could think of at that time.

To be clear - I like the idea of the Options object.  In Axis1, we could 
set properties on the Call object (like ServiceClient) which would be 
accessible via the MessageContext, but you'd need to do it for each 
Call.  Options is nicely split out so that you can reuse it for multiple 
ServiceClients - or at least it looks that way. :)

> I still do not see a huge difference between the current approach and
> what you suggest. It might reduce one object in the memory.

See below.

> Good point. But Options object will only have reference from a message
> context, so how can it be in the memory. We didn't see anything like
> that when we were doing memory profiling. Also how can a handler putting
> an object make a sudden change? Please explain a bit more.

The user code has a reference to the Options object, at least if you're 
reusing it.  See below again. :)

>> I'd really like us to consider changing this for 1.3 and into the future.
> 
> I still do not think we need to change this ;) BTW, what is the problem
> of the current approach? Obviously Ruchith's initial concern is now
> settled. Please help me to understand the problem, which is kinda hard
> using mailing lists.

A little code:

public class MyTestProgram {
   public static void main() throws Exception {
     Options opts = new Options();
     // ...initialize options...
     while (a_very_long_time) {
       ServiceClient client = new ServiceClient(opts);
       // ... do some more stuff ...
       client.sendReceive(...);
     }
   }
}

So every time around that loop, that Options object is becoming not only 
the storage for the stuff that the user put there, but also for any 
properties that the Handlers which run happen to put there.  Let's say 
you've got a Handler that can be deployed multiple times on a given 
flow; one that performs some function and then counts the number of 
times it has been invoked in a given message flow.  So it has code like:

   Integer count = (Integer)msgContext.getProperty("myCount");
   if (count == null) count = new Integer(0);
   count = new Integer(count.intValue() + 1);
   msgContext.setProperty("myCount");

Now, the idea is to record the number of invocations per MESSAGE, which 
is why it's writing to the *Message*Context.  You can see how the second 
time around the loop, the count becomes incorrect, right?  Because the 
property isn't per-MessageContext, it's per-Options.  Bad.

That's one kind of problem.  Another is that Handlers might use largish 
data structures and put them in MessageContext properties - any such 
structures won't get GC'ed for as long as the Options object is held... 
and that might be a long time (for instance I can imagine an Axis2 
system running in a continuous loop polling shop floor equipment 
checking for alert conditions - too hot, low on oil, etc).

Yet another problem is that I might want to reuse the same Options 
object across multiple ServiceClients - imagine a user doing that where 
they tried to do multiple invocations on multiple threads!  Two 
MessageContexts could be traversing different handler chains, with the 
exact same backing store for properties... I would be very surprised if 
that worked out well. :)

I see lots of problems with the current approach.  I don't (yet) see any 
problems which would arise out of fixing it.

Fixing it involves, IMO, separating the MC's property store from the 
Options bag, and making sure that getProperty() searches in the correct 
order.  There may be some more discussion needed to deal with use-cases 
where the user pulls properties back OUT of Options after invocation - 
do we have situations like that?

Thanks,
--Glen

---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org


Re: [Axis2] MessageContext and Options

Posted by Eran Chinthaka <ch...@opensource.lk>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Glen Daniels wrote:
> (note new subject line)
> 
> Hi Chinthaka:
> 
> Eran Chinthaka wrote:
>> Can you remember you raised the same question during the hackathon and I
>> answered it :). Anyway here it is again.
> 
> IIRC I asked about why Options were getting *copied* from place to place
> during MEPs, not why we're using the Options object as the
> MessageContext's property store... if I had realized that in Indiana,
> there would have been a longer conversation. :)
> 
>> Client should see Options object as a means to pass params to Axis2
>> engine and not as a place holder for his properties. When he passes that
> 
> Right, that sounds good!
> 
>> to MessageContext, we keep that underneath the message context as we do
>> not want the overhead of copying everything to a new data structure
>> inside the message context. Since this options object will be anyway
>> there inside the message context, and since it can store properties, we
>> didn't put another object to message context to store properties. We
> 
> OK, now this really does not sound good. :)  Why does this make sense? I
> thought the idea (please correct me if I'm wrong) of Options was that
> you could reuse it across invocations, keeping all the "application
> settable" information in one place.  If that's the case, then we are
> essentially cluttering a potentially long-lived object with references
> to properties that may have no meaning outside of a given invocation.
> 
> To put it another way - if Options is actually the exact same storage
> for properties as MessageContext, it doesn't seem like there was any
> point in creating the Options object at all.  We could have just left
> the MessageContext exposed to the user and they could use it's
> setTo()/setReplyTo()/etc methods.  Or if we wanted just an API shim for
> simplicity, we could have easily provided an Options interface which
> MessageContext would implement.

Well, I don't think exposing message context is an option as it has lots
of methods and complex. Yes we could come up with an alternative to
implement options interface to message context, but we didn't consider
that alternative when we were implementing it. The current method was
the best one that we could think of at that time.

I still do not see a huge difference between the current approach and
what you suggest. It might reduce one object in the memory.

> 
>> re-used the same options object. Yes, this is a deliberate design
>> decision. IIRC, me and Sanjiva (Deepal also, IIRC) had a long chat on
>> this, sent a proposal and did that.
> 
> I just spent a little while looking through the list archives.  I did
> see the "new client API" thread, in which Options was brought up as a
> way to capture "options to configure any MEP client".  But I did not see
> any discussion (until the code) of /replacing/ the MessageContext
> property bag with Options.  If I missed something, my apologies, but
> even if I did, I still don't agree with this design.
> 

>> See, what you are saying is in fact asking to copy the properties of the
>> options object to message context property object, leaving the room for
>> the user to re-use it. What we have done was asking the user to copy it
>> himself, if he needs to re-use it, but achieving possible performance
>> AND memory gains inside the engine.
> 
> First off, I'm not necessarily asking for any copying.  The obvious way
> to do this is to simply have the MC search the Options object if it
> doesn't find a property in its local store, just like it searches any
> other parent context.  I suspect that'll be quite sufficient, but if it
> turns out later ("don't prematurely optimize") that there's a real speed
> issue, then we can consider copying - which still shouldn't be very
> expensive (it would be odd if there were more than 50 or so things in
> there, I would guess).
> 
> As for "memory gains", I think *exactly* the opposite of what you're
> saying is true.  With the current system, any time a Handler puts an
> object into the MessageContext, there is suddenly a reference to that
> object that will hang around as long as the Options object hangs around,
> so for any kind of long-running client you've got a memory leak.  Long
> term storage is not the design center for MessageContexts!  MC's contain
> stuff that should GO AWAY after the processing of the message completes.
>  Stuff with longer lifespans should live in separate places.

Good point. But Options object will only have reference from a message
context, so how can it be in the memory. We didn't see anything like
that when we were doing memory profiling. Also how can a handler putting
an object make a sudden change? Please explain a bit more.

> 
>> Yes, it is by design and I don't see any issue with that. It is the way
>> you see it. The only problem is I am not sure we have written down this
>> somewhere, other than in a mail thread :(.
> 
> If you could point me towards the mail thread that would be great.

Ok, I will see. But my searching methodology is very primitive.

> 
> I'd really like us to consider changing this for 1.3 and into the future.

I still do not think we need to change this ;) BTW, what is the problem
of the current approach? Obviously Ruchith's initial concern is now
settled. Please help me to understand the problem, which is kinda hard
using mailing lists.

Thanks,
Chinthaka
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGgFSojON2uBzUhh8RAhqaAKCGoLB+kSsG/pPaM48e/zxedKGIzgCePSmK
PrSkIAY2QkQoXs8YsKdmrGA=
=WHVs
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: axis-dev-unsubscribe@ws.apache.org
For additional commands, e-mail: axis-dev-help@ws.apache.org