You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by sebb <se...@gmail.com> on 2012/09/03 23:47:33 UTC

Re: svn commit: r1380346 - /jmeter/trunk/src/components/org/apache/jmeter/config/RandomVariableConfig.java

On 3 September 2012 21:28,  <pm...@apache.org> wrote:
> Author: pmouawad
> Date: Mon Sep  3 20:28:41 2012
> New Revision: 1380346
>
> URL: http://svn.apache.org/viewvc?rev=1380346&view=rev
> Log:
> synchronized seems useless as all instances in same Thread Group get same value.

It's not useless.
Please don't just remove synch. without prior discussion.

> Should we add volatile ?

The test element is NoThreadClone, so is shared between threads.

Without sync. or volatile, the value written by one thread may never
be seen by another thread, or values may be seen out of order.
This is because the Java Memory Model allows threads to cache shared
values locally. Only if threads use the same lock is correct operation
guaranteed.
This is entirely separate from the locking which may be necessary to
protect multiple updates to one or more variables.

Using sync/volatile is necessary for safe data publication.
Unfortunately, most of the time the data does get updated, so such
bugs are rarely seen.

> Modified:
>     jmeter/trunk/src/components/org/apache/jmeter/config/RandomVariableConfig.java
>
> Modified: jmeter/trunk/src/components/org/apache/jmeter/config/RandomVariableConfig.java
> URL: http://svn.apache.org/viewvc/jmeter/trunk/src/components/org/apache/jmeter/config/RandomVariableConfig.java?rev=1380346&r1=1380345&r2=1380346&view=diff
> ==============================================================================
> --- jmeter/trunk/src/components/org/apache/jmeter/config/RandomVariableConfig.java (original)
> +++ jmeter/trunk/src/components/org/apache/jmeter/config/RandomVariableConfig.java Mon Sep  3 20:28:41 2012
> @@ -139,56 +139,56 @@ public class RandomVariableConfig extend
>      /**
>       * @return the minValue
>       */
> -    public synchronized String getMinimumValue() {
> +    public String getMinimumValue() {
>          return minimumValue;
>      }
>
>      /**
>       * @param minValue the minValue to set
>       */
> -    public synchronized void setMinimumValue(String minValue) {
> +    public void setMinimumValue(String minValue) {
>          this.minimumValue = minValue;
>      }
>
>      /**
>       * @return the maxvalue
>       */
> -    public synchronized String getMaximumValue() {
> +    public String getMaximumValue() {
>          return maximumValue;
>      }
>
>      /**
>       * @param maxvalue the maxvalue to set
>       */
> -    public synchronized void setMaximumValue(String maxvalue) {
> +    public void setMaximumValue(String maxvalue) {
>          this.maximumValue = maxvalue;
>      }
>
>      /**
>       * @return the variableName
>       */
> -    public synchronized String getVariableName() {
> +    public String getVariableName() {
>          return variableName;
>      }
>
>      /**
>       * @param variableName the variableName to set
>       */
> -    public synchronized void setVariableName(String variableName) {
> +    public void setVariableName(String variableName) {
>          this.variableName = variableName;
>      }
>
>      /**
>       * @return the randomSeed
>       */
> -    public synchronized String getRandomSeed() {
> +    public String getRandomSeed() {
>          return randomSeed;
>      }
>
>      /**
>       * @return the randomSeed as a long
>       */
> -    private synchronized long getRandomSeedAsLong() {
> +    private long getRandomSeedAsLong() {
>          long seed = 0;
>          if (randomSeed.length()==0){
>              seed = System.currentTimeMillis();
> @@ -206,33 +206,33 @@ public class RandomVariableConfig extend
>      /**
>       * @param randomSeed the randomSeed to set
>       */
> -    public synchronized void setRandomSeed(String randomSeed) {
> +    public void setRandomSeed(String randomSeed) {
>          this.randomSeed = randomSeed;
>      }
>
>      /**
>       * @return the perThread
>       */
> -    public synchronized boolean getPerThread() {
> +    public boolean getPerThread() {
>          return perThread;
>      }
>
>      /**
>       * @param perThread the perThread to set
>       */
> -    public synchronized void setPerThread(boolean perThread) {
> +    public void setPerThread(boolean perThread) {
>          this.perThread = perThread;
>      }
>      /**
>       * @return the outputFormat
>       */
> -    public synchronized String getOutputFormat() {
> +    public String getOutputFormat() {
>          return outputFormat;
>      }
>      /**
>       * @param outputFormat the outputFormat to set
>       */
> -    public synchronized void setOutputFormat(String outputFormat) {
> +    public void setOutputFormat(String outputFormat) {
>          this.outputFormat = outputFormat;
>      }
>
>
>

Re: svn commit: r1380346 - /jmeter/trunk/src/components/org/apache/jmeter/config/RandomVariableConfig.java

Posted by sebb <se...@gmail.com>.
On 3 September 2012 22:53, Philippe Mouawad <ph...@gmail.com> wrote:
> I debugged this one looking at NoThreadClone
> , all threads in same group write the same values for the same shared
> instance so what would be the issue ?
> I muqt say I don't understand this one.
> I had a doubt about putting volatile but what for ? Value will change for
> same value
>
> Can you explain it to me ?

If one thread writes a value to a shared variable, a different thread
which reads the variable may see an old value, or even, in the case of
long or double, a partially updated value (if the hardware or JVM does
not support atomic writes).

This is the same issue that means double-checked locking does not work
(unless using volatile, which is a form of memory lock).

For the full story, search for Java Memory Model.

Also worth reading the excellent book Java concurrency in practice -
"JCIP" - by Brian Goetz et al.

> Thanks
>
>
> On Monday, September 3, 2012, sebb wrote:
>
>> On 3 September 2012 21:28,  <pmouawad@apache.org <javascript:;>> wrote:
>> > Author: pmouawad
>> > Date: Mon Sep  3 20:28:41 2012
>> > New Revision: 1380346
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1380346&view=rev
>> > Log:
>> > synchronized seems useless as all instances in same Thread Group get
>> same value.
>>
>> It's not useless.
>> Please don't just remove synch. without prior discussion.
>>
>> > Should we add volatile ?
>>
>> The test element is NoThreadClone, so is shared between threads.
>>
>> Without sync. or volatile, the value written by one thread may never
>> be seen by another thread, or values may be seen out of order.
>> This is because the Java Memory Model allows threads to cache shared
>> values locally. Only if threads use the same lock is correct operation
>> guaranteed.
>> This is entirely separate from the locking which may be necessary to
>> protect multiple updates to one or more variables.
>>
>> Using sync/volatile is necessary for safe data publication.
>> Unfortunately, most of the time the data does get updated, so such
>> bugs are rarely seen.
>>
>> > Modified:
>> >
>> jmeter/trunk/src/components/org/apache/jmeter/config/RandomVariableConfig.java
>> >
>> > Modified:
>> jmeter/trunk/src/components/org/apache/jmeter/config/RandomVariableConfig.java
>> > URL:
>> http://svn.apache.org/viewvc/jmeter/trunk/src/components/org/apache/jmeter/config/RandomVariableConfig.java?rev=1380346&r1=1380345&r2=1380346&view=diff
>> >
>> ==============================================================================
>> > ---
>> jmeter/trunk/src/components/org/apache/jmeter/config/RandomVariableConfig.java
>> (original)
>> > +++
>> jmeter/trunk/src/components/org/apache/jmeter/config/RandomVariableConfig.java
>> Mon Sep  3 20:28:41 2012
>> > @@ -139,56 +139,56 @@ public class RandomVariableConfig extend
>> >      /**
>> >       * @return the minValue
>> >       */
>> > -    public synchronized String getMinimumValue() {
>> > +    public String getMinimumValue() {
>> >          return minimumValue;
>> >      }
>> >
>> >      /**
>> >       * @param minValue the minValue to set
>> >       */
>> > -    public synchronized void setMinimumValue(String minValue) {
>> > +    public void setMinimumValue(String minValue) {
>> >          this.minimumValue = minValue;
>> >      }
>> >
>> >      /**
>> >       * @return the maxvalue
>> >       */
>> > -    public synchronized String getMaximumValue() {
>> > +    public String getMaximumValue() {
>> >          return maximumValue;
>> >      }
>> >
>> >      /**
>> >       * @param maxvalue the maxvalue to set
>> >       */
>> > -    public synchronized void setMaximumValue(String maxvalue) {
>> > +    public void setMaximumValue(String maxvalue) {
>> >          this.maximumValue = maxvalue;
>> >      }
>> >
>> >      /**
>> >       * @return the variableName
>> >       */
>> > -    public synchronized String getVariableName() {
>> > +    public String getVariableName() {
>> >          return variableName;
>> >      }
>> >
>> >      /**
>> >       * @param variableName the variableName to set
>> >       */
>> > -    public synchronized void setVariableName(String variableName) {
>> > +    public void setVariableName(String variableName) {
>> >          this.variableName = variableName;
>> >      }
>> >
>> >      /**
>> >       * @return the randomSeed
>> >       */
>> > -    public synchronized String getRandomSeed() {
>> > +    public String getRandomSeed() {
>> >          return randomSeed;
>> >      }
>> >
>> >      /**
>> >       * @return the randomSeed as a long
>> >       */
>> > -    private synchronized long getRandomSeedAsLong() {
>> > +    private long getRandomSeedAsLong() {
>> >          long seed = 0;
>> >          if (randomSeed.length()==0){
>> >              seed = System.currentTimeMillis();
>> > @@ -206,33 +206,33 @@ public class RandomVariableConfig extend
>> >      /**
>> >       * @param randomSeed the randomSeed to set
>> >       */
>> > -    public synchronized void setRandomSeed(String randomSeed) {
>> > +    public void setRandomSeed(String randomSeed) {
>> >          this.randomSeed = randomSeed;
>> >      }
>> >
>> >      /**
>> >       * @return the perThread
>> >       */
>> > -    public synchronized boolean getPerThread() {
>> > +    public boolean getPerThread() {
>> >
>
>
>
> --
> Cordialement.
> Philippe Mouawad.

Re: svn commit: r1380346 - /jmeter/trunk/src/components/org/apache/jmeter/config/RandomVariableConfig.java

Posted by Philippe Mouawad <ph...@gmail.com>.
I debugged this one looking at NoThreadClone
, all threads in same group write the same values for the same shared
instance so what would be the issue ?
I muqt say I don't understand this one.
I had a doubt about putting volatile but what for ? Value will change for
same value

Can you explain it to me ?

Thanks


On Monday, September 3, 2012, sebb wrote:

> On 3 September 2012 21:28,  <pmouawad@apache.org <javascript:;>> wrote:
> > Author: pmouawad
> > Date: Mon Sep  3 20:28:41 2012
> > New Revision: 1380346
> >
> > URL: http://svn.apache.org/viewvc?rev=1380346&view=rev
> > Log:
> > synchronized seems useless as all instances in same Thread Group get
> same value.
>
> It's not useless.
> Please don't just remove synch. without prior discussion.
>
> > Should we add volatile ?
>
> The test element is NoThreadClone, so is shared between threads.
>
> Without sync. or volatile, the value written by one thread may never
> be seen by another thread, or values may be seen out of order.
> This is because the Java Memory Model allows threads to cache shared
> values locally. Only if threads use the same lock is correct operation
> guaranteed.
> This is entirely separate from the locking which may be necessary to
> protect multiple updates to one or more variables.
>
> Using sync/volatile is necessary for safe data publication.
> Unfortunately, most of the time the data does get updated, so such
> bugs are rarely seen.
>
> > Modified:
> >
> jmeter/trunk/src/components/org/apache/jmeter/config/RandomVariableConfig.java
> >
> > Modified:
> jmeter/trunk/src/components/org/apache/jmeter/config/RandomVariableConfig.java
> > URL:
> http://svn.apache.org/viewvc/jmeter/trunk/src/components/org/apache/jmeter/config/RandomVariableConfig.java?rev=1380346&r1=1380345&r2=1380346&view=diff
> >
> ==============================================================================
> > ---
> jmeter/trunk/src/components/org/apache/jmeter/config/RandomVariableConfig.java
> (original)
> > +++
> jmeter/trunk/src/components/org/apache/jmeter/config/RandomVariableConfig.java
> Mon Sep  3 20:28:41 2012
> > @@ -139,56 +139,56 @@ public class RandomVariableConfig extend
> >      /**
> >       * @return the minValue
> >       */
> > -    public synchronized String getMinimumValue() {
> > +    public String getMinimumValue() {
> >          return minimumValue;
> >      }
> >
> >      /**
> >       * @param minValue the minValue to set
> >       */
> > -    public synchronized void setMinimumValue(String minValue) {
> > +    public void setMinimumValue(String minValue) {
> >          this.minimumValue = minValue;
> >      }
> >
> >      /**
> >       * @return the maxvalue
> >       */
> > -    public synchronized String getMaximumValue() {
> > +    public String getMaximumValue() {
> >          return maximumValue;
> >      }
> >
> >      /**
> >       * @param maxvalue the maxvalue to set
> >       */
> > -    public synchronized void setMaximumValue(String maxvalue) {
> > +    public void setMaximumValue(String maxvalue) {
> >          this.maximumValue = maxvalue;
> >      }
> >
> >      /**
> >       * @return the variableName
> >       */
> > -    public synchronized String getVariableName() {
> > +    public String getVariableName() {
> >          return variableName;
> >      }
> >
> >      /**
> >       * @param variableName the variableName to set
> >       */
> > -    public synchronized void setVariableName(String variableName) {
> > +    public void setVariableName(String variableName) {
> >          this.variableName = variableName;
> >      }
> >
> >      /**
> >       * @return the randomSeed
> >       */
> > -    public synchronized String getRandomSeed() {
> > +    public String getRandomSeed() {
> >          return randomSeed;
> >      }
> >
> >      /**
> >       * @return the randomSeed as a long
> >       */
> > -    private synchronized long getRandomSeedAsLong() {
> > +    private long getRandomSeedAsLong() {
> >          long seed = 0;
> >          if (randomSeed.length()==0){
> >              seed = System.currentTimeMillis();
> > @@ -206,33 +206,33 @@ public class RandomVariableConfig extend
> >      /**
> >       * @param randomSeed the randomSeed to set
> >       */
> > -    public synchronized void setRandomSeed(String randomSeed) {
> > +    public void setRandomSeed(String randomSeed) {
> >          this.randomSeed = randomSeed;
> >      }
> >
> >      /**
> >       * @return the perThread
> >       */
> > -    public synchronized boolean getPerThread() {
> > +    public boolean getPerThread() {
> >



-- 
Cordialement.
Philippe Mouawad.