[GitHub] jmeter issue #256: Bug60637 better statistics summary report

2017-01-25 Thread ra0077
Github user ra0077 commented on the issue:

https://github.com/apache/jmeter/pull/256
  
Hi Philippe,

Thanks to your feedback
I will integrate your modification.

If it's ok for all, I will modify the documentation and merge it

One question, do I need to modify the release note with a screenshot?

Antonio



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


Re: svn commit: r1780141 - /jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/config/gui/JavaConfigGui.java

2017-01-25 Thread Felix Schumacher


Am 25. Januar 2017 08:31:32 MEZ schrieb pmoua...@apache.org:
>Author: pmouawad
>Date: Wed Jan 25 07:31:31 2017
>New Revision: 1780141
>
>URL: http://svn.apache.org/viewvc?rev=1780141&view=rev
>Log:
>Ignore sonar false positive
>
>Modified:
>jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/config/gui/JavaConfigGui.java
>
>Modified:
>jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/config/gui/JavaConfigGui.java
>URL:
>http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/config/gui/JavaConfigGui.java?rev=1780141&r1=1780140&r2=1780141&view=diff
>==
>---
>jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/config/gui/JavaConfigGui.java
>(original)
>+++
>jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/config/gui/JavaConfigGui.java
>Wed Jan 25 07:31:31 2017
>@@ -266,7 +266,7 @@ public class JavaConfigGui extends Abstr
>JavaSamplerClient client = (JavaSamplerClient) Class.forName(className,
>true,
> Thread.currentThread().getContextClassLoader()).newInstance();
> return client != null;

Can client be null, or would an exception be thrown?

>-} catch (Exception ex) {
>+} catch (Exception ex) { // NOSONAR We already log this

I think sonar complains, as we are loosing the information from the exception 
here.

Should we log it here and remove the log from the calling function?

Felix
 
> return false;
> }
> }


Re: svn commit: r1780141 - /jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/config/gui/JavaConfigGui.java

2017-01-25 Thread Philippe Mouawad
On Wed, Jan 25, 2017 at 10:01 AM, Felix Schumacher <
felix.schumac...@internetallee.de> wrote:

>
>
> Am 25. Januar 2017 08:31:32 MEZ schrieb pmoua...@apache.org:
> >Author: pmouawad
> >Date: Wed Jan 25 07:31:31 2017
> >New Revision: 1780141
> >
> >URL: http://svn.apache.org/viewvc?rev=1780141&view=rev
> >Log:
> >Ignore sonar false positive
> >
> >Modified:
> >jmeter/trunk/src/protocol/java/org/apache/jmeter/
> protocol/java/config/gui/JavaConfigGui.java
> >
> >Modified:
> >jmeter/trunk/src/protocol/java/org/apache/jmeter/
> protocol/java/config/gui/JavaConfigGui.java
> >URL:
> >http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/
> java/org/apache/jmeter/protocol/java/config/gui/JavaConfigGui.java?rev=
> 1780141&r1=1780140&r2=1780141&view=diff
> >===
> ===
> >---
> >jmeter/trunk/src/protocol/java/org/apache/jmeter/
> protocol/java/config/gui/JavaConfigGui.java
> >(original)
> >+++
> >jmeter/trunk/src/protocol/java/org/apache/jmeter/
> protocol/java/config/gui/JavaConfigGui.java
> >Wed Jan 25 07:31:31 2017
> >@@ -266,7 +266,7 @@ public class JavaConfigGui extends Abstr
> >JavaSamplerClient client = (JavaSamplerClient) Class.forName(className,
> >true,
> > Thread.currentThread().getContextClassLoader()).newInstance();
> > return client != null;
>
> Can client be null, or would an exception be thrown?
>
No but it is just to cast to JavaSamplerClient (needed) and use the local
variable to avoid another warning.
Do you have another idea ?

>
> >-} catch (Exception ex) {
> >+} catch (Exception ex) { // NOSONAR We already log this
>
> I think sonar complains, as we are loosing the information from the
> exception here.
>


>
> Should we log it here and remove the log from the calling function?
>
Yes, will fix it this way

>
> Felix
>
> > return false;
> > }
> > }
>



-- 
Cordialement.
Philippe Mouawad.


[GitHub] jmeter issue #253: Fix issue with annotation and implement remarks from last...

2017-01-25 Thread max3163
Github user max3163 commented on the issue:

https://github.com/apache/jmeter/pull/253
  
Plse re-consider your last commit on the ANNOTATION side where you put tags 
as a field... 
Tags is a tag not a field.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


Re: svn commit: r1780141 - /jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/config/gui/JavaConfigGui.java

2017-01-25 Thread Felix Schumacher


Am 25. Januar 2017 13:31:12 MEZ schrieb Philippe Mouawad 
:
>On Wed, Jan 25, 2017 at 10:01 AM, Felix Schumacher <
>felix.schumac...@internetallee.de> wrote:
>
>>
>>
>> Am 25. Januar 2017 08:31:32 MEZ schrieb pmoua...@apache.org:
>> >Author: pmouawad
>> >Date: Wed Jan 25 07:31:31 2017
>> >New Revision: 1780141
>> >
>> >URL: http://svn.apache.org/viewvc?rev=1780141&view=rev
>> >Log:
>> >Ignore sonar false positive
>> >
>> >Modified:
>> >jmeter/trunk/src/protocol/java/org/apache/jmeter/
>> protocol/java/config/gui/JavaConfigGui.java
>> >
>> >Modified:
>> >jmeter/trunk/src/protocol/java/org/apache/jmeter/
>> protocol/java/config/gui/JavaConfigGui.java
>> >URL:
>> >http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/
>>
>java/org/apache/jmeter/protocol/java/config/gui/JavaConfigGui.java?rev=
>> 1780141&r1=1780140&r2=1780141&view=diff
>> >===
>> ===
>> >---
>> >jmeter/trunk/src/protocol/java/org/apache/jmeter/
>> protocol/java/config/gui/JavaConfigGui.java
>> >(original)
>> >+++
>> >jmeter/trunk/src/protocol/java/org/apache/jmeter/
>> protocol/java/config/gui/JavaConfigGui.java
>> >Wed Jan 25 07:31:31 2017
>> >@@ -266,7 +266,7 @@ public class JavaConfigGui extends Abstr
>> >JavaSamplerClient client = (JavaSamplerClient)
>Class.forName(className,
>> >true,
>> >
>Thread.currentThread().getContextClassLoader()).newInstance();
>> > return client != null;
>>
>> Can client be null, or would an exception be thrown?
>>
>No but it is just to cast to JavaSamplerClient (needed) and use the
>local
>variable to avoid another warning.
>Do you have another idea ?

Return true? My comment was about the comparison to null. The cast is obvious 
and necessary.

>
>>
>> >-} catch (Exception ex) {
>> >+} catch (Exception ex) { // NOSONAR We already log this
>>
>> I think sonar complains, as we are loosing the information from the
>> exception here.
>>
>
>
>>
>> Should we log it here and remove the log from the calling function?
>>
>Yes, will fix it this way

Thanks,
Felix

>
>>
>> Felix
>>
>> > return false;
>> > }
>> > }
>>


Re: svn commit: r1780178 - /jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/config/gui/JavaConfigGui.java

2017-01-25 Thread Felix Schumacher


Am 25. Januar 2017 13:32:56 MEZ schrieb pmoua...@apache.org:
>Author: pmouawad
>Date: Wed Jan 25 12:32:56 2017
>New Revision: 1780178
>
>URL: http://svn.apache.org/viewvc?rev=1780178&view=rev
>Log:
>Bug 55652 - JavaSampler silently resets classname if class can not be
>found
>Fix Sonar error in a better way , thanks Felix
>Bugzilla Id: 55652
>
>Modified:
>jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/config/gui/JavaConfigGui.java
>
>Modified:
>jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/config/gui/JavaConfigGui.java
>URL:
>http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/config/gui/JavaConfigGui.java?rev=1780178&r1=1780177&r2=1780178&view=diff
>==
>---
>jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/config/gui/JavaConfigGui.java
>(original)
>+++
>jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/config/gui/JavaConfigGui.java
>Wed Jan 25 12:32:56 2017
>@@ -248,7 +248,6 @@ public class JavaConfigGui extends Abstr
> }
> 
> if(!classOk(className)) {
>-log.error("Error setting class:'"+className+"' in
>JavaSampler "+getName()+", check for a missing jar in your jmeter
>'search_paths' and 'plugin_dependency_paths' properties");
> warningLabel.setVisible(true);
> } else {
> warningLabel.setVisible(false);

Great, now we could just set the label directly without the if statement :)

warning label.set visible(!classOk(class name))

Regards,
Felix
>@@ -261,12 +260,14 @@ public class JavaConfigGui extends Abstr
>  * @param className String class name
>* @return true if class is ok (exist in classpath and instanceof {@link
>JavaSamplerClient}
>  */
>-private static boolean classOk(String className) {
>+private boolean classOk(String className) {
> try {
>JavaSamplerClient client = (JavaSamplerClient) Class.forName(className,
>true,
> Thread.currentThread().getContextClassLoader()).newInstance();
> return client != null;
>-} catch (Exception ex) { // NOSONAR We already log this
>+} catch (Exception ex) {
>+log.error("Error creating class:'"+className+"' in
>JavaSampler "+getName()
>++", check for a missing jar in your jmeter
>'search_paths' and 'plugin_dependency_paths' properties");
> return false;
> }
> }


[GitHub] jmeter issue #253: Fix issue with annotation and implement remarks from last...

2017-01-25 Thread max3163
Github user max3163 commented on the issue:

https://github.com/apache/jmeter/pull/253
  
I made some test on the last nigthly ( by the way, the 
apache-jmeter-r1780178.zip miss one librairie => httpcore-nio-4.4.6 ) 
When the test takes few second ( less than 5 second ) only the " ended " 
annotation is sending. 
I don't understand why ...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


Deprecate HTML Link Parser and HTTP URL Re-writing Modifier

2017-01-25 Thread Philippe Mouawad
Hello,

What do you think of deprecating HTML Link Parser and HTTP URL Re-writing
Modifier?

   - I am not sure they are widely used
   - Their design does not suit well for performance as :
  - they require Previous Response, as a consequence, they do not work
  in Distributed mode
  - they use a Dom parser + Tidy
  - They are  based on old HTML parser so I don't think they work
   properly with new HTML 5 code
   - They suffer from old bugs:
  - https://bz.apache.org/bugzilla/show_bug.cgi?id=17252
  - https://bz.apache.org/bugzilla/show_bug.cgi?id=59943 => It was not
  reported on this element, but investigation showed it was affected

We could ask question on User mailing list and twitter to see if it's used.


Regards
Philippe M.
@philmdot


Re: Deprecate HTML Link Parser and HTTP URL Re-writing Modifier

2017-01-25 Thread Andrey Pokhilko
+1 to drop them, they seems to not align to general purpose of core JMeter

Andrey Pokhilko

On 25.01.2017 19:44, Philippe Mouawad wrote:
> Hello,
>
> What do you think of deprecating HTML Link Parser and HTTP URL Re-writing
> Modifier?
>
>- I am not sure they are widely used
>- Their design does not suit well for performance as :
>   - they require Previous Response, as a consequence, they do not work
>   in Distributed mode
>   - they use a Dom parser + Tidy
>   - They are  based on old HTML parser so I don't think they work
>properly with new HTML 5 code
>- They suffer from old bugs:
>   - https://bz.apache.org/bugzilla/show_bug.cgi?id=17252
>   - https://bz.apache.org/bugzilla/show_bug.cgi?id=59943 => It was not
>   reported on this element, but investigation showed it was affected
>
> We could ask question on User mailing list and twitter to see if it's used.
>
>
> Regards
> Philippe M.
> @philmdot
>



Re: Deprecate HTML Link Parser and HTTP URL Re-writing Modifier

2017-01-25 Thread Milamber


HTML Link Parser element is probably rarely used and can be deprecated imho

HTTP URL Re-writing element can be useful for testing webapp without 
cookie manager but probably for a very old app of the time when the 
cookie is appeared into the browsers... Probably it's can be deprecated too.



On 25/01/2017 16:44, Philippe Mouawad wrote:

Hello,

What do you think of deprecating HTML Link Parser and HTTP URL Re-writing
Modifier?

- I am not sure they are widely used
- Their design does not suit well for performance as :
   - they require Previous Response, as a consequence, they do not work
   in Distributed mode
   - they use a Dom parser + Tidy
   - They are  based on old HTML parser so I don't think they work
properly with new HTML 5 code
- They suffer from old bugs:
   - https://bz.apache.org/bugzilla/show_bug.cgi?id=17252
   - https://bz.apache.org/bugzilla/show_bug.cgi?id=59943 => It was not
   reported on this element, but investigation showed it was affected

We could ask question on User mailing list and twitter to see if it's used.


Regards
Philippe M.
@philmdot





[GitHub] jmeter issue #253: Fix issue with annotation and implement remarks from last...

2017-01-25 Thread pmouawad
Github user pmouawad commented on the issue:

https://github.com/apache/jmeter/pull/253
  
@max3163 , I've fixed the missing httpcore-nio.
For the bug, it's because you schedule every 5s, so test ends before and 
closes the Sender and just guarantees end is sent.
We could fix it but I'm not sure it's useful, why would anybody do a 5s 
load test ?

Regarding your note on Annotation, I don't understand why you want me to 
rollback. 
As I wrote, I think we should expose tags so that users can put whatever 
they want.
I didn't fully understand what @Wyatts  wrote but I think it's worth a 
discussion with all dev team before going further.

Regards



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] jmeter issue #253: Fix issue with annotation and implement remarks from last...

2017-01-25 Thread max3163
Github user max3163 commented on the issue:

https://github.com/apache/jmeter/pull/253
  
When we flush metrics in the teardownTest method, why the start annotations 
is not send too ? 

For my remark, I just want to put tags as tag and not as field. The user 
can put whatever he want too and it's more logic int influxdb sense to have 
Tags as tag.

I just propose this : 

influxdbMetricsManager.addMetric(EVENTS_FOR_ANNOTATION, TAG_APPLICATION + 
application + ",title=ApacheJMeter,tags=" + 
AbstractInfluxdbMetricsSender.tagToStringValue(tags) , 
"text=\"" +  AbstractInfluxdbMetricsSender
.fieldToStringValue(testTitle + " started") + "\"" 
);

And add a new parameter call tags as parameter of the backend.
If you are OK, I will push a new PR in this sense ?

I think it's important to let influxdb index this parameter (only tag are 
indexed ) and just keep the text value as field.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


Re: svn commit: r1780141 - /jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/config/gui/JavaConfigGui.java

2017-01-25 Thread Philippe Mouawad
On Wed, Jan 25, 2017 at 4:05 PM, Felix Schumacher <
felix.schumac...@internetallee.de> wrote:

>
>
> Am 25. Januar 2017 13:31:12 MEZ schrieb Philippe Mouawad <
> philippe.moua...@gmail.com>:
> >On Wed, Jan 25, 2017 at 10:01 AM, Felix Schumacher <
> >felix.schumac...@internetallee.de> wrote:
> >
> >>
> >>
> >> Am 25. Januar 2017 08:31:32 MEZ schrieb pmoua...@apache.org:
> >> >Author: pmouawad
> >> >Date: Wed Jan 25 07:31:31 2017
> >> >New Revision: 1780141
> >> >
> >> >URL: http://svn.apache.org/viewvc?rev=1780141&view=rev
> >> >Log:
> >> >Ignore sonar false positive
> >> >
> >> >Modified:
> >> >jmeter/trunk/src/protocol/java/org/apache/jmeter/
> >> protocol/java/config/gui/JavaConfigGui.java
> >> >
> >> >Modified:
> >> >jmeter/trunk/src/protocol/java/org/apache/jmeter/
> >> protocol/java/config/gui/JavaConfigGui.java
> >> >URL:
> >> >http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/
> >>
> >java/org/apache/jmeter/protocol/java/config/gui/JavaConfigGui.java?rev=
> >> 1780141&r1=1780140&r2=1780141&view=diff
> >> >===
> >> ===
> >> >---
> >> >jmeter/trunk/src/protocol/java/org/apache/jmeter/
> >> protocol/java/config/gui/JavaConfigGui.java
> >> >(original)
> >> >+++
> >> >jmeter/trunk/src/protocol/java/org/apache/jmeter/
> >> protocol/java/config/gui/JavaConfigGui.java
> >> >Wed Jan 25 07:31:31 2017
> >> >@@ -266,7 +266,7 @@ public class JavaConfigGui extends Abstr
> >> >JavaSamplerClient client = (JavaSamplerClient)
> >Class.forName(className,
> >> >true,
> >> >
> >Thread.currentThread().getContextClassLoader()).newInstance();
> >> > return client != null;
> >>
> >> Can client be null, or would an exception be thrown?
> >>
> >No but it is just to cast to JavaSamplerClient (needed) and use the
> >local
> >variable to avoid another warning.
> >Do you have another idea ?
>
> Return true? My comment was about the comparison to null. The cast is
> obvious and necessary.
>
But Sonar will say unused var no ?
I commited, feel free to fix as you like, I won't complain :-)

>
> >
> >>
> >> >-} catch (Exception ex) {
> >> >+} catch (Exception ex) { // NOSONAR We already log this
> >>
> >> I think sonar complains, as we are loosing the information from the
> >> exception here.
> >>
> >
> >
> >>
> >> Should we log it here and remove the log from the calling function?
> >>
> >Yes, will fix it this way
>
> Thanks,
> Felix
>
> >
> >>
> >> Felix
> >>
> >> > return false;
> >> > }
> >> > }
> >>
>



-- 
Cordialement.
Philippe Mouawad.


[GitHub] jmeter issue #253: Fix issue with annotation and implement remarks from last...

2017-01-25 Thread pmouawad
Github user pmouawad commented on the issue:

https://github.com/apache/jmeter/pull/253
  
@max3163 ,
Ok for your proposal.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] jmeter issue #257: Static Analysis Fixes

2017-01-25 Thread pmouawad
Github user pmouawad commented on the issue:

https://github.com/apache/jmeter/pull/257
  
@ham1 , Thanks a lot for your contribution.
Looks globally good for me.
I have just one remark, I wouldn't "optimize" the org.apache.jmeter.control 
package classes.
This part of JMeter is one of the most complex and less readable.
We need by the way to increase our Test coverage on these.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] jmeter issue #247: Checks for listener output file existence

2017-01-25 Thread pmouawad
Github user pmouawad commented on the issue:

https://github.com/apache/jmeter/pull/247
  
Hello,
I reviewed this PR and I have some blocker remarks so I didn't merge it:
- I think code in StandardJMeterEngine  and DistributedRunner is duplicated
- I wouldn't put any dependency on java.awt in those 2 classes.
- deleteResultFile should not be static but I don't know how to make it 
better

Regards
Philippe


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


Bug 60646 - Workbench : Save it by default

2017-01-25 Thread Philippe Mouawad
Hello,
Anybody is against switching this ?

Thank you
Regards


Re: Bug 60646 - Workbench : Save it by default

2017-01-25 Thread Antonio Gomes Rodrigues
+1 to save it by default

Antonio

2017-01-25 20:32 GMT+01:00 Philippe Mouawad :

> Hello,
> Anybody is against switching this ?
>
> Thank you
> Regards
>


Support HTTP/2 protocol

2017-01-25 Thread Philippe Mouawad
Hello
I'd like to start a thread on this particular item for which an enhancement
exists:

   - https://bz.apache.org/bugzilla/show_bug.cgi?id=59847

The aim of this thread is to discuss, throw ideas on how we could implement
this in JMeter.

Oleg K. from HttpComponents project has nicely proposed to help on it.

I see at least 2 parts in this item:

   - The Sampler
   - The Recorder



*Sampler:*
We have 2 options:

   - build a usual "synchronous" sampler similar to HTTP:
  - Is this realistic ?
  - Does it perform well ?
  - + : It should not be too complex
   - build a new "Asynchronous sampler":
  - Is this realistic ?
  - + We could gain more performance
  - - It is a huge piece of work as we need to change JMeter model

*Recorder:*

I think we need to introduce a new more generic Recorder as the current
Test Script Recorder is too tightly linked to HTTP 1.X protocol


Regards
Philippe M.
@philmdot


[GitHub] jmeter pull request #256: Bug60637 better statistics summary report

2017-01-25 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/jmeter/pull/256


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


Re: Deprecate HTML Link Parser and HTTP URL Re-writing Modifier

2017-01-25 Thread Antonio Gomes Rodrigues
+1 to drop them

Antonio

2017-01-25 18:28 GMT+01:00 Milamber :

>
> HTML Link Parser element is probably rarely used and can be deprecated imho
>
> HTTP URL Re-writing element can be useful for testing webapp without
> cookie manager but probably for a very old app of the time when the cookie
> is appeared into the browsers... Probably it's can be deprecated too.
>
>
>
> On 25/01/2017 16:44, Philippe Mouawad wrote:
>
>> Hello,
>>
>> What do you think of deprecating HTML Link Parser and HTTP URL Re-writing
>> Modifier?
>>
>> - I am not sure they are widely used
>> - Their design does not suit well for performance as :
>>- they require Previous Response, as a consequence, they do not
>> work
>>in Distributed mode
>>- they use a Dom parser + Tidy
>>- They are  based on old HTML parser so I don't think they work
>> properly with new HTML 5 code
>> - They suffer from old bugs:
>>- https://bz.apache.org/bugzilla/show_bug.cgi?id=17252
>>- https://bz.apache.org/bugzilla/show_bug.cgi?id=59943 => It was
>> not
>>reported on this element, but investigation showed it was affected
>>
>> We could ask question on User mailing list and twitter to see if it's
>> used.
>>
>>
>> Regards
>> Philippe M.
>> @philmdot
>>
>>
>


[GitHub] jmeter issue #257: Static Analysis Fixes

2017-01-25 Thread pmouawad
Github user pmouawad commented on the issue:

https://github.com/apache/jmeter/pull/257
  
Hello, 
Another question I have.
I see you replace StringBuilder by '+' concat.
Is there a reason for this ?
Reading:
- 
http://www.pellegrino.link/2015/08/22/string-concatenation-with-java-8.html

I don't understand that their performance is better.
Do you have another reference ?
Thanks


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


Review of HttpMetricsSender, TextGraphiteMetricsSender and PickleGraphiteMetricsSender

2017-01-25 Thread Philippe Mouawad
Hello,
There was a similar synchro bug in those 3 classes as reported in:

   - https://bz.apache.org/bugzilla/show_bug.cgi?id=60648

CopyOnWriteArrayList does not seem to be a good option.

I fixed them with regular Synchronization but I wonder if there is not a
better way using Java 8.

But I am not sure.

I read this:

-
http://blog.takipi.com/java-8-stampedlocks-vs-readwritelocks-and-synchronized/
-
https://docs.oracle.com/javase/8/docs/technotes/guides/concurrency/changes8.html

My intention was to have a thread safe collections (without using
Collections.synchronized...) and have a method to atomically "drain" rows
from the list in a local variable , this way synchro would not have been
needed.


Is there a way through streams to do that  ?
Sorry for my stupid questions.
Thanks for help
Regards


[GitHub] jmeter issue #257: Static Analysis Fixes

2017-01-25 Thread ham1
Github user ham1 commented on the issue:

https://github.com/apache/jmeter/pull/257
  
Thanks for looking at the PR and commenting.

I will take another look at the changes to `org.apache.jmeter.control` and 
either revert or justify them.
I'd gladly add more tests if we move to Spock ;)

The reason for replacing `StringBuilder` with `+` is for **readability**.

That article shows, that for the simple code i.e. not building a string 
inside loops etc. they are the same i.e. the JVM generates a `StringBuilder` 
for the simple `+` code.
I believe they have the same performance but for me reading:

`"nodeList[" + i + "] " + nodeList.item(i)`
is much easier than:
`new StringBuilder("nodeList[").append(i).append("] 
").append(nodeList.item(i)).toString()`

and

`"Row: " + row`
much easier than:
`new StringBuilder("Row: ").append(row).toString()`

My change in `InfluxdbBackendListenerClient` might be a tiny bit slower due 
to the initial capacity that the JVM might use the default of 16 causing some 
resizing of buffers.

Hope that makes sense.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


Re: Review of HttpMetricsSender, TextGraphiteMetricsSender and PickleGraphiteMetricsSender

2017-01-25 Thread Felix Schumacher


Am 25. Januar 2017 23:28:48 MEZ schrieb Philippe Mouawad 
:
>Hello,
>There was a similar synchro bug in those 3 classes as reported in:
>
>   - https://bz.apache.org/bugzilla/show_bug.cgi?id=60648
>
>CopyOnWriteArrayList does not seem to be a good option.
>
>I fixed them with regular Synchronization but I wonder if there is not
>a
>better way using Java 8.

Would a ConcurrentLinkedQueue fit your need?

Felix

>
>But I am not sure.
>
>I read this:
>
>-
>http://blog.takipi.com/java-8-stampedlocks-vs-readwritelocks-and-synchronized/
>-
>https://docs.oracle.com/javase/8/docs/technotes/guides/concurrency/changes8.html
>
>My intention was to have a thread safe collections (without using
>Collections.synchronized...) and have a method to atomically "drain"
>rows
>from the list in a local variable , this way synchro would not have
>been
>needed.
>
>
>Is there a way through streams to do that  ?
>Sorry for my stupid questions.
>Thanks for help
>Regards