You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@jmeter.apache.org by bu...@apache.org on 2012/08/21 00:30:54 UTC

[Bug 53749] New: TestListener interface could perhaps be split up

https://issues.apache.org/bugzilla/show_bug.cgi?id=53749

          Priority: P2
            Bug ID: 53749
          Assignee: issues@jmeter.apache.org
           Summary: TestListener interface could perhaps be split up
          Severity: enhancement
    Classification: Unclassified
                OS: All
          Reporter: sebb@apache.org
          Hardware: All
            Status: NEW
           Version: unspecified
         Component: Main
           Product: JMeter

The TestListener interface includes the following method names

testStarted
testEnded
testIterationStart

Now testIterationStart is rather different from testStarted/testEnded; it is
called much more frequently, and is probably much less used.

So it might make more sense to split the interface into two super-interfaces.

This would allow classes to drop testIterationStart if they don't need it, and
allow (for example) JMeterThread to only store references to code that actually
needs to implement testIterationStart.

-- 
You are receiving this mail because:
You are the assignee for the bug.

Re: [Bug 53749] TestListener interface could perhaps be split up

Posted by sebb <se...@gmail.com>.
On 22 August 2012 11:29, Milamber <mi...@apache.org> wrote:
> On Wed, Aug 22, 2012 at 11:16 AM, Philippe Mouawad <
> philippe.mouawad@gmail.com> wrote:
>
>> Hello,
>> It seems to me lot of impact with low improvement but if you want to do
>> it...
>>
>> I remember we had a discussion on that kind of impacting changes about
>> logger implementation .
>> I wanted to switch to one not deprecated as ours and as impact was high we
>> abandoned it.
>>
>> As we are now in these big changes (static final, interface cleanup ...  )
>> Sebb, milamber is it ok for you if I start migration to commons-logging ?
>>
>
>
> Why commons-loggings (not updated since 2008)?

AIUI it's not been updated since it works; there has been no need to update it.

> Log4J ?

> or directly java.util.logging.*?

That's broken, according to what I read.

>
>
>
>
>
>>
>> Regards
>> Philippe
>> On Wednesday, August 22, 2012, wrote:
>>
>> > https://issues.apache.org/bugzilla/show_bug.cgi?id=53749
>> >
>> > --- Comment #3 from Sebb <sebb@apache.org <javascript:;>> ---
>> > Just done a check.
>> >
>> > There are about 40 implementations of testIterationStart, only 4 of which
>> > have
>> > any code; the rest are required because of the interface.
>> >
>> > One of the real implementations -
>> > RemoteSampleListenerImpl.testIterationStart -
>> > is probably redundant anyway because TIS has not been implemented for
>> > client-server.
>> >
>> > --
>> > You are receiving this mail because:
>> > You are the assignee for the bug.
>> >
>>
>>
>> --
>> Cordialement.
>> Philippe Mouawad.
>>

Re: [Bug 53749] TestListener interface could perhaps be split up

Posted by Philippe Mouawad <ph...@gmail.com>.
Initial thread:
http://mail-archives.apache.org/mod_mbox/jmeter-dev/201201.mbox/%3CCAH9fUpYWBNH4bMzw0SwZEwN_GKKy1qe42k-i%3DZb6_kZo2FLLXQ%40mail.gmail.com%3E

On Wed, Aug 22, 2012 at 12:49 PM, Philippe Mouawad <
philippe.mouawad@gmail.com> wrote:

> At frst i voted for logback + slf4j but it was after discussion il the dev
> list that commons-logging seemed a better option.
>
> Regards
> Philippe
>
>
> On Wednesday, August 22, 2012, Milamber wrote:
>
>> On Wed, Aug 22, 2012 at 11:16 AM, Philippe Mouawad <
>> philippe.mouawad@gmail.com> wrote:
>>
>> > Hello,
>> > It seems to me lot of impact with low improvement but if you want to do
>> > it...
>> >
>> > I remember we had a discussion on that kind of impacting changes about
>> > logger implementation .
>> > I wanted to switch to one not deprecated as ours and as impact was high
>> we
>> > abandoned it.
>> >
>> > As we are now in these big changes (static final, interface cleanup ...
>>  )
>> > Sebb, milamber is it ok for you if I start migration to commons-logging
>> ?
>> >
>>
>>
>> Why commons-loggings (not updated since 2008)? Log4J ? or directly
>> java.util.logging.*?
>>
>>
>>
>>
>>
>> >
>> > Regards
>> > Philippe
>> > On Wednesday, August 22, 2012, wrote:
>> >
>> > > https://issues.apache.org/bugzilla/show_bug.cgi?id=53749
>> > >
>> > > --- Comment #3 from Sebb <sebb@apache.org <javascript:;>> ---
>> > > Just done a check.
>> > >
>> > > There are about 40 implementations of testIterationStart, only 4 of
>> which
>> > > have
>> > > any code; the rest are required because of the interface.
>> > >
>> > > One of the real implementations -
>> > > RemoteSampleListenerImpl.testIterationStart -
>> > > is probably redundant anyway because TIS has not been implemented for
>> > > client-server.
>> > >
>> > > --
>> > > You are receiving this mail because:
>> > > You are the assignee for the bug.
>> > >
>> >
>> >
>> > --
>> > Cordialement.
>> > Philippe Mouawad.
>> >
>>
>
>
> --
> Cordialement.
> Philippe Mouawad.
>
>
>
>


-- 
Cordialement.
Philippe Mouawad.

Re: [Bug 53749] TestListener interface could perhaps be split up

Posted by Philippe Mouawad <ph...@gmail.com>.
At frst i voted for logback + slf4j but it was after discussion il the dev
list that commons-logging seemed a better option.

Regards
Philippe

On Wednesday, August 22, 2012, Milamber wrote:

> On Wed, Aug 22, 2012 at 11:16 AM, Philippe Mouawad <
> philippe.mouawad@gmail.com <javascript:;>> wrote:
>
> > Hello,
> > It seems to me lot of impact with low improvement but if you want to do
> > it...
> >
> > I remember we had a discussion on that kind of impacting changes about
> > logger implementation .
> > I wanted to switch to one not deprecated as ours and as impact was high
> we
> > abandoned it.
> >
> > As we are now in these big changes (static final, interface cleanup ...
>  )
> > Sebb, milamber is it ok for you if I start migration to commons-logging ?
> >
>
>
> Why commons-loggings (not updated since 2008)? Log4J ? or directly
> java.util.logging.*?
>
>
>
>
>
> >
> > Regards
> > Philippe
> > On Wednesday, August 22, 2012, wrote:
> >
> > > https://issues.apache.org/bugzilla/show_bug.cgi?id=53749
> > >
> > > --- Comment #3 from Sebb <sebb@apache.org <javascript:;><javascript:;>> ---
> > > Just done a check.
> > >
> > > There are about 40 implementations of testIterationStart, only 4 of
> which
> > > have
> > > any code; the rest are required because of the interface.
> > >
> > > One of the real implementations -
> > > RemoteSampleListenerImpl.testIterationStart -
> > > is probably redundant anyway because TIS has not been implemented for
> > > client-server.
> > >
> > > --
> > > You are receiving this mail because:
> > > You are the assignee for the bug.
> > >
> >
> >
> > --
> > Cordialement.
> > Philippe Mouawad.
> >
>


-- 
Cordialement.
Philippe Mouawad.

Re: [Bug 53749] TestListener interface could perhaps be split up

Posted by Milamber <mi...@apache.org>.
On Wed, Aug 22, 2012 at 11:16 AM, Philippe Mouawad <
philippe.mouawad@gmail.com> wrote:

> Hello,
> It seems to me lot of impact with low improvement but if you want to do
> it...
>
> I remember we had a discussion on that kind of impacting changes about
> logger implementation .
> I wanted to switch to one not deprecated as ours and as impact was high we
> abandoned it.
>
> As we are now in these big changes (static final, interface cleanup ...  )
> Sebb, milamber is it ok for you if I start migration to commons-logging ?
>


Why commons-loggings (not updated since 2008)? Log4J ? or directly
java.util.logging.*?





>
> Regards
> Philippe
> On Wednesday, August 22, 2012, wrote:
>
> > https://issues.apache.org/bugzilla/show_bug.cgi?id=53749
> >
> > --- Comment #3 from Sebb <sebb@apache.org <javascript:;>> ---
> > Just done a check.
> >
> > There are about 40 implementations of testIterationStart, only 4 of which
> > have
> > any code; the rest are required because of the interface.
> >
> > One of the real implementations -
> > RemoteSampleListenerImpl.testIterationStart -
> > is probably redundant anyway because TIS has not been implemented for
> > client-server.
> >
> > --
> > You are receiving this mail because:
> > You are the assignee for the bug.
> >
>
>
> --
> Cordialement.
> Philippe Mouawad.
>

Re: [Bug 53749] TestListener interface could perhaps be split up

Posted by Philippe Mouawad <ph...@gmail.com>.
Hello,
It seems to me lot of impact with low improvement but if you want to do
it...

I remember we had a discussion on that kind of impacting changes about
logger implementation .
I wanted to switch to one not deprecated as ours and as impact was high we
abandoned it.

As we are now in these big changes (static final, interface cleanup ...  )
Sebb, milamber is it ok for you if I start migration to commons-logging ?

Regards
Philippe
On Wednesday, August 22, 2012, wrote:

> https://issues.apache.org/bugzilla/show_bug.cgi?id=53749
>
> --- Comment #3 from Sebb <sebb@apache.org <javascript:;>> ---
> Just done a check.
>
> There are about 40 implementations of testIterationStart, only 4 of which
> have
> any code; the rest are required because of the interface.
>
> One of the real implementations -
> RemoteSampleListenerImpl.testIterationStart -
> is probably redundant anyway because TIS has not been implemented for
> client-server.
>
> --
> You are receiving this mail because:
> You are the assignee for the bug.
>


-- 
Cordialement.
Philippe Mouawad.

[Bug 53749] TestListener interface could perhaps be split up

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53749

--- Comment #3 from Sebb <se...@apache.org> ---
Just done a check. 

There are about 40 implementations of testIterationStart, only 4 of which have
any code; the rest are required because of the interface.

One of the real implementations - RemoteSampleListenerImpl.testIterationStart -
is probably redundant anyway because TIS has not been implemented for
client-server.

-- 
You are receiving this mail because:
You are the assignee for the bug.

Re: [Bug 53749] TestListener interface could perhaps be split up

Posted by sebb <se...@gmail.com>.
On 22 August 2012 11:14, Philippe Mouawad <ph...@gmail.com> wrote:
> Hello,
> It seems to me lot of impact with low improvement but if you want to do
> it...

I'm still evaluating whether it's worthwhile or not.

It's not yet clear how big the improvement will be, as not all test
elements use TestListener.
For a test made up mainly of elements that use it but not
testIterationListener, it would save 80-90 of entries in the
Collection in each thread, so it has a the potential to save quite a
bit of memory and processing time.

> I remember we had a discussion on that kind of impacting changes about
> logger implementation .
> I wanted to switch to one not deprecated as ours and as impact was high we
> abandoned it.
>
> As we are now in these big changes (static final, interface cleanup ...  )
> Sebb, milamber is it ok for you if I start migration to commons-logging ?

This should be the subject of a separate e-mail thread please.

> Regards
> Philippe
>
> On Wednesday, August 22, 2012, wrote:
>
>> https://issues.apache.org/bugzilla/show_bug.cgi?id=53749
>>
>> --- Comment #2 from Sebb <sebb@apache.org <javascript:;>> ---
>> (In reply to comment #1)
>> > Should we close this one ?
>>
>> No.
>>
>> I hit an initial problem, but I'm still hopeful it can be resolved.
>>
>> For reference, the issue is:
>>
>> The code that implements the interface behaviour currently only looks for
>> TestListener.
>> All the code needs to be updated to look for the new interfaces instead;
>> this
>> involves quite a lot of classes. So far I have found:
>>
>> ConvertListeners
>> StandardJMeterEngine
>> FunctionParser
>> RemoteSamplerListenerImpl
>> ProxyControl
>>
>> There may perhaps be some other processing I've yet to find.
>>
>> I created separate threadStart/threadEnded interfaces, as some classes only
>> need one or the other, but then realised that makes the Remote classes
>> tricky
>> to update.
>>
>> At that point I decided to revert the initial changes.
>>
>> Further investigation shows that the Remote code does not support
>> testIterationStart currently, which should make the implementation much
>> easier.
>> [It would be expensive in network bandwidth, but luckily none of the
>> Remoteable
>> classes need to use it]
>>
>> --
>> You are receiving this mail because:
>> You are the assignee for the bug.
>>
>
>
> --
> Cordialement.
> Philippe Mouawad.

Re: [Bug 53749] TestListener interface could perhaps be split up

Posted by Philippe Mouawad <ph...@gmail.com>.
Hello,
It seems to me lot of impact with low improvement but if you want to do
it...

I remember we had a discussion on that kind of impacting changes about
logger implementation .
I wanted to switch to one not deprecated as ours and as impact was high we
abandoned it.

As we are now in these big changes (static final, interface cleanup ...  )
Sebb, milamber is it ok for you if I start migration to commons-logging ?

Regards
Philippe

On Wednesday, August 22, 2012, wrote:

> https://issues.apache.org/bugzilla/show_bug.cgi?id=53749
>
> --- Comment #2 from Sebb <sebb@apache.org <javascript:;>> ---
> (In reply to comment #1)
> > Should we close this one ?
>
> No.
>
> I hit an initial problem, but I'm still hopeful it can be resolved.
>
> For reference, the issue is:
>
> The code that implements the interface behaviour currently only looks for
> TestListener.
> All the code needs to be updated to look for the new interfaces instead;
> this
> involves quite a lot of classes. So far I have found:
>
> ConvertListeners
> StandardJMeterEngine
> FunctionParser
> RemoteSamplerListenerImpl
> ProxyControl
>
> There may perhaps be some other processing I've yet to find.
>
> I created separate threadStart/threadEnded interfaces, as some classes only
> need one or the other, but then realised that makes the Remote classes
> tricky
> to update.
>
> At that point I decided to revert the initial changes.
>
> Further investigation shows that the Remote code does not support
> testIterationStart currently, which should make the implementation much
> easier.
> [It would be expensive in network bandwidth, but luckily none of the
> Remoteable
> classes need to use it]
>
> --
> You are receiving this mail because:
> You are the assignee for the bug.
>


-- 
Cordialement.
Philippe Mouawad.

[Bug 53749] TestListener interface could perhaps be split up

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53749

--- Comment #2 from Sebb <se...@apache.org> ---
(In reply to comment #1)
> Should we close this one ?

No.

I hit an initial problem, but I'm still hopeful it can be resolved.

For reference, the issue is:

The code that implements the interface behaviour currently only looks for
TestListener. 
All the code needs to be updated to look for the new interfaces instead; this
involves quite a lot of classes. So far I have found:

ConvertListeners
StandardJMeterEngine
FunctionParser
RemoteSamplerListenerImpl
ProxyControl

There may perhaps be some other processing I've yet to find.

I created separate threadStart/threadEnded interfaces, as some classes only
need one or the other, but then realised that makes the Remote classes tricky
to update.

At that point I decided to revert the initial changes.

Further investigation shows that the Remote code does not support
testIterationStart currently, which should make the implementation much easier.
[It would be expensive in network bandwidth, but luckily none of the Remoteable
classes need to use it]

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 53749] TestListener interface could perhaps be split up

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53749

Philippe Mouawad <p....@ubik-ingenierie.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |p.mouawad@ubik-ingenierie.c
                   |                            |om

--- Comment #1 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
Should we close this one ?

-- 
You are receiving this mail because:
You are the assignee for the bug.

[Bug 53749] TestListener interface could perhaps be split up

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53749

Sebb <se...@apache.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #4 from Sebb <se...@apache.org> ---
Fixed in SVN:

URL: http://svn.apache.org/viewvc?rev=1376300&view=rev
Log:
TestListener interface could perhaps be split up.
Bugzilla Id: 53749

Added:
   
jmeter/trunk/src/core/org/apache/jmeter/testelement/TestIterationListener.java 
 (with props)
    jmeter/trunk/src/core/org/apache/jmeter/testelement/TestListener.java
      - copied, changed from r1375825,
jmeter/trunk/src/core/org/apache/jmeter/testelement/TestListener.java
    jmeter/trunk/src/core/org/apache/jmeter/testelement/TestStateListener.java 
 (with props)
Modified:
    jmeter/trunk/src/components/org/apache/jmeter/config/KeystoreConfig.java
   
jmeter/trunk/src/components/org/apache/jmeter/control/ThroughputController.java
   
jmeter/trunk/src/components/org/apache/jmeter/timers/ConstantThroughputTimer.java
    jmeter/trunk/src/components/org/apache/jmeter/timers/SyncTimer.java
    jmeter/trunk/src/core/org/apache/jmeter/JMeter.java
    jmeter/trunk/src/core/org/apache/jmeter/engine/ConvertListeners.java
    jmeter/trunk/src/core/org/apache/jmeter/engine/StandardJMeterEngine.java
    jmeter/trunk/src/core/org/apache/jmeter/engine/util/FunctionParser.java
    jmeter/trunk/src/core/org/apache/jmeter/gui/MainFrame.java
    jmeter/trunk/src/core/org/apache/jmeter/reporters/ResultCollector.java
    jmeter/trunk/src/core/org/apache/jmeter/reporters/Summariser.java
    jmeter/trunk/src/core/org/apache/jmeter/samplers/RemoteListenerWrapper.java
   
jmeter/trunk/src/core/org/apache/jmeter/samplers/RemoteSampleListenerImpl.java
   
jmeter/trunk/src/core/org/apache/jmeter/samplers/RemoteTestListenerWrapper.java
    jmeter/trunk/src/core/org/apache/jmeter/testelement/TestPlan.java
    jmeter/trunk/src/core/org/apache/jmeter/threads/JMeterThread.java
    jmeter/trunk/src/core/org/apache/jmeter/util/BeanShellTestElement.java
    jmeter/trunk/src/functions/org/apache/jmeter/functions/StringFromFile.java
   
jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CacheManager.java
   
jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/control/CookieManager.java
   
jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/modifier/ParamModifier.java
   
jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/proxy/ProxyControl.java
   
jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java
   
jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java
   
jmeter/trunk/src/protocol/jdbc/org/apache/jmeter/protocol/jdbc/AbstractJDBCTestElement.java
   
jmeter/trunk/src/protocol/jdbc/org/apache/jmeter/protocol/jdbc/config/DataSourceElement.java
   
jmeter/trunk/src/protocol/jms/org/apache/jmeter/protocol/jms/sampler/JMSSampler.java
   
jmeter/trunk/src/protocol/jms/org/apache/jmeter/protocol/jms/sampler/PublisherSampler.java
   
jmeter/trunk/src/protocol/jms/org/apache/jmeter/protocol/jms/sampler/SubscriberSampler.java
   
jmeter/trunk/src/protocol/ldap/org/apache/jmeter/protocol/ldap/sampler/LDAPExtSampler.java
    jmeter/trunk/src/reports/org/apache/jmeter/testelement/ReportPlan.java
    jmeter/trunk/xdocs/changes.xml

-- 
You are receiving this mail because:
You are the assignee for the bug.