jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r1205639 - in /jmeter/trunk: src/components/org/apache/jmeter/timers/BarrierWrapper.java src/components/org/apache/jmeter/timers/SyncTimer.java xdocs/changes.xml
Date Thu, 24 Nov 2011 09:24:00 GMT
On 24 November 2011 07:49, Philippe Mouawad <philippe.mouawad@gmail.com> wrote:
> On Wed, Nov 23, 2011 at 11:54 PM, sebb <sebbaz@gmail.com> wrote:
>
>> On 23 November 2011 22:33,  <pmouawad@apache.org> wrote:
>> > Author: pmouawad
>> > Date: Wed Nov 23 22:33:32 2011
>> > New Revision: 1205639
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1205639&view=rev
>> > Log:
>> > Bug 52183 - SyncTimer could be improved (performance+reliability)
>> >
>> > Added:
>> >
>>  jmeter/trunk/src/components/org/apache/jmeter/timers/BarrierWrapper.java
>> (with props)
>> >    jmeter/trunk/src/components/org/apache/jmeter/timers/SyncTimer.java
>>
>> What happened to the previous versions of SyncTimer?
>>
>> I am sorry doing some renamings I think I destroyed history (I will try to
> get it back)

One way to do this might be to replace it with the last version of the
previous incarnation; then re-apply the changes since then (with the
same log messages).

There's some info in the SVN docn. about recovering deleted files (but
it's not all that clearly written as I remember).

>
>> All the history seems to have been lost.
>>
>> > Modified:
>> >    jmeter/trunk/xdocs/changes.xml
>> >
>> > Added:
>> jmeter/trunk/src/components/org/apache/jmeter/timers/BarrierWrapper.java
>> > URL:
>> http://svn.apache.org/viewvc/jmeter/trunk/src/components/org/apache/jmeter/timers/BarrierWrapper.java?rev=1205639&view=auto
>> >
>> ==============================================================================
>> > ---
>> jmeter/trunk/src/components/org/apache/jmeter/timers/BarrierWrapper.java
>> (added)
>> > +++
>> jmeter/trunk/src/components/org/apache/jmeter/timers/BarrierWrapper.java
>> Wed Nov 23 22:33:32 2011
>> > @@ -0,0 +1,107 @@
>> > +/*
>> > + * Licensed to the Apache Software Foundation (ASF) under one or more
>> > + * contributor license agreements.  See the NOTICE file distributed with
>> > + * this work for additional information regarding copyright ownership.
>> > + * The ASF licenses this file to You under the Apache License, Version
>> 2.0
>> > + * (the "License"); you may not use this file except in compliance with
>> > + * the License.  You may obtain a copy of the License at
>> > + *
>> > + *   http://www.apache.org/licenses/LICENSE-2.0
>> > + *
>> > + * Unless required by applicable law or agreed to in writing, software
>> > + * distributed under the License is distributed on an "AS IS" BASIS,
>> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>> implied.
>> > + * See the License for the specific language governing permissions and
>> > + * limitations under the License.
>> > + *
>> > + */
>> > +
>> > +package org.apache.jmeter.timers;
>> > +
>> > +import java.util.concurrent.BrokenBarrierException;
>> > +import java.util.concurrent.CyclicBarrier;
>> > +import java.util.concurrent.TimeUnit;
>> > +import java.util.concurrent.TimeoutException;
>> > +
>> > +/**
>> > + * Wrapper to {@link CyclicBarrier} to allow lazy init of CyclicBarrier
>> when SyncTimer is configured with 0
>> > + */
>> > +class BarrierWrapper implements Cloneable {
>> > +       private CyclicBarrier barrier;
>>
>> This field is not properly synchronised.
>> It's not sufficient to synch. write access; read access must also be synch.
>>
>>
> I think it is OK like this.
> BarrierWrapper is just for use by SyncTimer.
> As CyclicBarrier handles synchronisation and field is initialized in
> constructor and setup is synchronized I don't think we need more.
>

Synch. is necessary not just for the thread that writes the field;
threads that read it also need to synch. on the same lock.

However, given that setup also reads the field, and setup should
either be called by all threads in the group or none (depending on the
group size), on further reflection I think the code is OK. But in the
context of the class alone, it looks wrong.

To prevent complaints from Findbugs, it might be an idea to move the
synch. to the calling method, and document the synch. requirement in
the setup Javadoc.

I need to think about that some more; it can be fixed (if necessary)
once the SyncTimer history has been restored.

Maybe even move the wrapper class to be a static class in SyncTimer,
as I'm not sure it is useful elsewhere.

>
>
>> > +
>> > +       /**
>> > +        *
>> > +        */
>> > +       public BarrierWrapper() {
>> > +               this.barrier = null;
>> > +       }
>> > +
>> > +       /**
>> > +        * @param parties Number of parties
>> > +        */
>> > +       public BarrierWrapper(int parties) {
>> > +               this.barrier = new CyclicBarrier(parties);
>> > +       }
>> > +       /**
>> > +        * @param parties Number of parties
>> > +        */
>> > +       public synchronized void setup(int parties) {
>> > +               if(this.barrier== null) {
>> > +                       this.barrier = new CyclicBarrier(parties);
>> > +               }
>> > +       }
>> > +
>> > +       /**
>> > +        * @see CyclicBarrier#await()
>> > +        * @return int
>> > +        * @throws InterruptedException
>> > +        * @throws BrokenBarrierException
>> > +        * @see java.util.concurrent.CyclicBarrier#await()
>> > +        */
>> > +       public int await() throws InterruptedException,
>> BrokenBarrierException {
>> > +               return barrier.await();
>> > +       }
>> > +
>> > +       /**
>> > +        * @param timeout Timeout
>> > +        * @param unit {@link TimeUnit}
>> > +        * @throws InterruptedException
>> > +        * @throws BrokenBarrierException
>> > +        * @throws TimeoutException
>> > +        * @see java.util.concurrent.CyclicBarrier#await(long,
>> java.util.concurrent.TimeUnit)
>> > +        */
>> > +       public int await(long timeout, TimeUnit unit) throws
>> InterruptedException,
>> > +                       BrokenBarrierException, TimeoutException
{
>> > +               return barrier.await(timeout, unit);
>> > +       }
>> > +
>> > +       /**
>> > +        * @see java.util.concurrent.CyclicBarrier#isBroken()
>> > +        */
>> > +       public boolean isBroken() {
>> > +               return barrier.isBroken();
>> > +       }
>> > +
>> > +       /**
>> > +        * @see java.util.concurrent.CyclicBarrier#reset()
>> > +        */
>> > +       public void reset() {
>> > +               barrier.reset();
>> > +       }
>> > +
>> > +       /**
>> > +        * @see java.lang.Object#clone()
>> > +        */
>> > +       @Override
>> > +       protected Object clone()  {
>> > +               BarrierWrapper barrierWrapper=  null;
>> > +               try {
>> > +                       barrierWrapper = (BarrierWrapper) super.clone();
>> > +                       barrierWrapper.barrier = this.barrier;
>> > +               } catch (CloneNotSupportedException e) {
>> > +                       //Cannot happen
>> > +               }
>> > +               return barrierWrapper;
>> > +       }
>> > +
>> > +}
>> > \ No newline at end of file
>> >
>> > Propchange:
>> jmeter/trunk/src/components/org/apache/jmeter/timers/BarrierWrapper.java
>> >
>> ------------------------------------------------------------------------------
>> >    svn:mime-type = text/plain
>> >
>> > Added:
>> jmeter/trunk/src/components/org/apache/jmeter/timers/SyncTimer.java
>> > URL:
>> http://svn.apache.org/viewvc/jmeter/trunk/src/components/org/apache/jmeter/timers/SyncTimer.java?rev=1205639&view=auto
>> >
>> ==============================================================================
>> > --- jmeter/trunk/src/components/org/apache/jmeter/timers/SyncTimer.java
>> (added)
>> > +++ jmeter/trunk/src/components/org/apache/jmeter/timers/SyncTimer.java
>> Wed Nov 23 22:33:32 2011
>> > @@ -0,0 +1,156 @@
>> > +/*
>> > + * Licensed to the Apache Software Foundation (ASF) under one or more
>> > + * contributor license agreements.  See the NOTICE file distributed with
>> > + * this work for additional information regarding copyright ownership.
>> > + * The ASF licenses this file to You under the Apache License, Version
>> 2.0
>> > + * (the "License"); you may not use this file except in compliance with
>> > + * the License.  You may obtain a copy of the License at
>> > + *
>> > + * http://www.apache.org/licenses/LICENSE-2.0
>> > + *
>> > + * Unless required by applicable law or agreed to in writing, software
>> > + * distributed under the License is distributed on an "AS IS" BASIS,
>> WITHOUT
>> > + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See
>> the
>> > + * License for the specific language governing permissions and
>> limitations
>> > + * under the License.
>> > + *
>> > + */
>> > +
>> > +package org.apache.jmeter.timers;
>> > +
>> > +import java.io.Serializable;
>> > +import java.util.concurrent.BrokenBarrierException;
>> > +
>> > +import org.apache.jmeter.engine.event.LoopIterationEvent;
>> > +import org.apache.jmeter.testbeans.TestBean;
>> > +import org.apache.jmeter.testelement.AbstractTestElement;
>> > +import org.apache.jmeter.testelement.TestListener;
>> > +import org.apache.jmeter.testelement.ThreadListener;
>> > +import org.apache.jmeter.threads.JMeterContextService;
>> > +
>> > +/**
>> > + * The purpose of the SyncTimer is to block threads until X number of
>> threads
>> > + * have been blocked, and then they are all released at once. A
>> SyncTimer can
>> > + * thus create large instant loads at various points of the test plan.
>> > + *
>> > + */
>> > +public class SyncTimer extends AbstractTestElement implements Timer,
>> Serializable, TestBean, TestListener, ThreadListener {
>> > +    private static final long serialVersionUID = 2;
>> > +
>> > +    private BarrierWrapper barrier;
>> > +
>> > +    private int groupSize;
>> > +
>> > +    // Ensure transient object is created by the server
>> > +    private Object readResolve(){
>> > +       createBarrier();
>> > +        return this;
>> > +    }
>> > +
>> > +    /**
>> > +     * @return Returns the numThreads.
>> > +     */
>> > +    public int getGroupSize() {
>> > +        return groupSize;
>> > +    }
>> > +
>> > +    /**
>> > +     * @param numThreads
>> > +     *            The numThreads to set.
>> > +     */
>> > +    public void setGroupSize(int numThreads) {
>> > +        this.groupSize = numThreads;
>> > +    }
>> > +
>> > +    /**
>> > +     * {@inheritDoc}
>> > +     */
>> > +    public long delay() {
>> > +       if(getGroupSize()>=0) {
>> > +               int arrival = 0;
>> > +               try {
>> > +                               arrival = this.barrier.await();
>> > +                       } catch (InterruptedException e) {
>> > +                               return 0;
>> > +                       } catch (BrokenBarrierException e) {
>> > +                               return 0;
>> > +                       } finally {
>> > +                               if(arrival == 0) {
>> > +                                       barrier.reset();
>> > +                               }
>> > +                       }
>> > +       }
>> > +        return 0;
>> > +    }
>> > +
>> > +    /**
>> > +     * We have to control the cloning process because we need some
>> cross-thread
>> > +     * communication if our synctimers are to be able to determine when
>> to block
>> > +     * and when to release.
>> > +     */
>> > +    @Override
>> > +    public Object clone() {
>> > +        SyncTimer newTimer = (SyncTimer) super.clone();
>> > +        newTimer.barrier = barrier;
>> > +        return newTimer;
>> > +    }
>> > +
>> > +    /**
>> > +     * {@inheritDoc}
>> > +     */
>> > +    public void testEnded() {
>> > +        this.testEnded(null);
>> > +    }
>> > +
>> > +    /**
>> > +     * Reset timerCounter
>> > +     */
>> > +    public void testEnded(String host) {
>> > +       createBarrier();
>> > +    }
>> > +
>> > +    /**
>> > +     * {@inheritDoc}
>> > +     */
>> > +    public void testStarted() {
>> > +        testStarted(null);
>> > +    }
>> > +
>> > +    /**
>> > +     * Reset timerCounter
>> > +     */
>> > +    public void testStarted(String host) {
>> > +        createBarrier();
>> > +    }
>> > +
>> > +       @Override
>> > +       public void testIterationStart(LoopIterationEvent event) {
>> > +               // NOOP
>> > +       }
>> > +
>> > +       /**
>> > +        *
>> > +        */
>> > +       private void createBarrier() {
>> > +               if(getGroupSize() == 0) {
>> > +                       // Lazy init
>> > +                       this.barrier = new BarrierWrapper();
>> > +        } else {
>> > +               this.barrier = new BarrierWrapper(getGroupSize());
>> > +        }
>> > +       }
>> > +
>> > +       @Override
>> > +       public void threadStarted() {
>> > +               int numThreadsInGroup =
>> JMeterContextService.getContext().getThreadGroup().getNumThreads();
>> > +               if(getGroupSize() == 0) {
>> > +                       // Unique Barrier creation ensured by
>> synchronized setup
>> > +                       this.barrier.setup(numThreadsInGroup);
>> > +        }
>> > +       }
>> > +
>> > +       @Override
>> > +       public void threadFinished() {
>> > +               // NOOP
>> > +       }
>> > +}
>> > \ No newline at end of file
>> >
>> > Modified: jmeter/trunk/xdocs/changes.xml
>> > URL:
>> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1205639&r1=1205638&r2=1205639&view=diff
>> >
>> ==============================================================================
>> > --- jmeter/trunk/xdocs/changes.xml (original)
>> > +++ jmeter/trunk/xdocs/changes.xml Wed Nov 23 22:33:32 2011
>> > @@ -188,6 +188,7 @@ This behaviour can be changed with prope
>> >  <h3>Timers, Assertions, Config, Pre- &amp; Post-Processors</h3>
>> >  <ul>
>> >  <li>Bug 52128 - Add JDBC pre- and post-processor</li>
>> > +<li>Bug 52183 - SyncTimer could be improved
>> (performance+reliability)</li>
>> >  </ul>
>> >
>> >  <h3>Functions</h3>
>> >
>> >
>> >
>>
>
>
>
> --
> Cordialement.
> Philippe Mouawad.
>

Mime
View raw message