You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jmeter-dev@jakarta.apache.org by "BAZLEY, Sebastian" <Se...@london.sema.slb.com> on 2004/03/22 14:00:49 UTC

RE: Is JMeterContextService.getContext() thread safe? Should it b e such?

Not sure it should be read-only.
The context is useful (and already used?) for communicating information
within a thread.

I recently added a version of getContext() to AbstractTestElement that
caches the context variable.

The idea being that the TestElements are all cloned per thread, and so
there's no point fetching the context each time. As this can only be called
from one thread, there's no need to synchronize the method - though the
methods in JMeterContextService probably ought to be synchronized.

But I've just had a thought - are any test elements not cloned per-thread,
but called from multiple threads? If any so, these would need to get the
context directly from JMeterContextService.

JMeterContextService should probably now be converted to use ThreadLocal.
This is designed for the purpose, and has better performance than it did
originally.

S.
-----Original Message-----
From: Michael Stover [mailto:mstover1@apache.org]
Sent: 22 March 2004 12:30
To: JMeter Developers List
Subject: Re: Is JMeterContextService.getContext() thread safe? Should it be
such?


To my way of thinking, getContext() should be entirely read-only and the
initialization of thread contexts should be provided for prior to the
test beginning.  That would avoid an unnecessary synchronization point.

-Mike

On Mon, 2004-03-22 at 03:48, Michal Kostrzewa wrote:
> Strange thing, I didn't notice your mail from wenesday, sorry :(
>
> > How would that cause a problem?  The Map that holds the thread contexts
> > is only being read, not written to.  So, many threads can ask for their
> > context simultaneously, this should not be a problem.  Unless you can
> > think of a scenario that is problematic?
> >
>
> Not exactly, because there is contextMap.get and then contextMap.put if
> context was null - classic test-and-set problem. But I was also partly
wrong,
> because indeed the get and put works on the same context element in only
one
> thread.
>
> But this is not all - Is the HashMap used in this class synchronized? I
think
> it's not (HashTable is). So I think getContext should be synchronized
anyway.
>
>
> ...
> static private Map contextMap = new HashMap();
> ...
> static public JMeterContext getContext()
>     {
>         init();
>         JMeterContext context =
>             (JMeterContext)
contextMap.get(Thread.currentThread().getName());
>         if (context == null)
>         {
>             context = new JMeterContext();
>             setContext(context);
>         }
>         return context;
>     }
>
> In my problem (bug 27744) this was not the case. I fixed this bug and I'm
> going to commit the patch (It is in the bugzilla already but nobody tested
> it :(((
> :))))
> best regards,
> Michał Kostrzewa
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: jmeter-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: jmeter-dev-help@jakarta.apache.org
--
Michael Stover <ms...@apache.org>
Apache Software Foundation


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


___________________________________________________________________________

This e-mail and the documents attached are confidential and intended solely
for the addressee; it may also be privileged. If you receive this e-mail in
error, please notify the sender immediately and destroy it. As its integrity
cannot be secured on the Internet, the Atos Origin group liability cannot be
triggered for the message content. Although the sender endeavours to maintain
a computer virus-free network, the sender does not warrant that this
transmission is virus-free and will not be liable for any damages resulting
from any virus transmitted. 
___________________________________________________________________________


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


RE: Is JMeterContextService.getContext() thread safe? Should it b e such?

Posted by mstover <ms...@rochester.rr.com>.
I'm not talking about making the context itself read-only, just the
getContext() method that retrieves the context according to the current
thread.  Each context should be accessible only to a single thread via
this strategy and shouldn't need synchronization for read or write
operations.

I wouldn't think caching the context in the test elements would be a
good idea.  I think there are elements that are not cloned per thread -
such as resultcollectors (I could be wrong - I don't think cloning them
would break anything).  There may be others.  However, I would be in
favor of enforcing cloning-per-thread.  I think it's cleaner
conceptually, and one can always (and should) override the clone method
in your test element to carry over required state instance members from
clone to clone.  This is a better way to ensure proper behavior than
using a marker interface to prevent cloning (my original strategy -
oops).

Relevant Classes:
	org.apache.jmeter.engine.TreeCloner - you'll see it does avoid cloning
objects marked with the NoThreadClone interface
	org.apache.jmeter.modifiers.CounterConfig - implements NoThreadClone. 
	org.apache.jmeter.control.ThroughputController - accomplishes the same
intent by overriding clone()

If we guaranteed everything cloned, then caching the context in each
element wouldn't be a problem.

Using ThreadLocal inside the JMeterContextService to provide the
per-thread access to contexts sounds like a good idea.

-Mike

On Mon, 2004-03-22 at 08:00, BAZLEY, Sebastian wrote:
> Not sure it should be read-only.
> The context is useful (and already used?) for communicating information
> within a thread.
> 
> I recently added a version of getContext() to AbstractTestElement that
> caches the context variable.
> 
> The idea being that the TestElements are all cloned per thread, and so
> there's no point fetching the context each time. As this can only be called
> from one thread, there's no need to synchronize the method - though the
> methods in JMeterContextService probably ought to be synchronized.
> 
> But I've just had a thought - are any test elements not cloned per-thread,
> but called from multiple threads? If any so, these would need to get the
> context directly from JMeterContextService.
> 
> JMeterContextService should probably now be converted to use ThreadLocal.
> This is designed for the purpose, and has better performance than it did
> originally.
> 
> S.
> -----Original Message-----
> From: Michael Stover [mailto:mstover1@apache.org]
> Sent: 22 March 2004 12:30
> To: JMeter Developers List
> Subject: Re: Is JMeterContextService.getContext() thread safe? Should it be
> such?
> 
> 
> To my way of thinking, getContext() should be entirely read-only and the
> initialization of thread contexts should be provided for prior to the
> test beginning.  That would avoid an unnecessary synchronization point.
> 
> -Mike
> 
> On Mon, 2004-03-22 at 03:48, Michal Kostrzewa wrote:
> > Strange thing, I didn't notice your mail from wenesday, sorry :(
> >
> > > How would that cause a problem?  The Map that holds the thread contexts
> > > is only being read, not written to.  So, many threads can ask for their
> > > context simultaneously, this should not be a problem.  Unless you can
> > > think of a scenario that is problematic?
> > >
> >
> > Not exactly, because there is contextMap.get and then contextMap.put if
> > context was null - classic test-and-set problem. But I was also partly
> wrong,
> > because indeed the get and put works on the same context element in only
> one
> > thread.
> >
> > But this is not all - Is the HashMap used in this class synchronized? I
> think
> > it's not (HashTable is). So I think getContext should be synchronized
> anyway.
> >
> >
> > ...
> > static private Map contextMap = new HashMap();
> > ...
> > static public JMeterContext getContext()
> >     {
> >         init();
> >         JMeterContext context =
> >             (JMeterContext)
> contextMap.get(Thread.currentThread().getName());
> >         if (context == null)
> >         {
> >             context = new JMeterContext();
> >             setContext(context);
> >         }
> >         return context;
> >     }
> >
> > In my problem (bug 27744) this was not the case. I fixed this bug and I'm
> > going to commit the patch (It is in the bugzilla already but nobody tested
> > it :(((
> > :))))
> > best regards,
> > Michał Kostrzewa
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: jmeter-dev-unsubscribe@jakarta.apache.org
> > For additional commands, e-mail: jmeter-dev-help@jakarta.apache.org
> --
> Michael Stover <ms...@apache.org>
> Apache Software Foundation
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: jmeter-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: jmeter-dev-help@jakarta.apache.org
> 
> 
> ___________________________________________________________________________
> 
> This e-mail and the documents attached are confidential and intended solely
> for the addressee; it may also be privileged. If you receive this e-mail in
> error, please notify the sender immediately and destroy it. As its integrity
> cannot be secured on the Internet, the Atos Origin group liability cannot be
> triggered for the message content. Although the sender endeavours to maintain
> a computer virus-free network, the sender does not warrant that this
> transmission is virus-free and will not be liable for any damages resulting
> from any virus transmitted. 
> ___________________________________________________________________________
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: jmeter-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: jmeter-dev-help@jakarta.apache.org
> 


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