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 2016/08/18 15:43:37 UTC

[Bug 60017] New: Performance optimisation : Nullify comment field

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

            Bug ID: 60017
           Summary: Performance optimisation : Nullify comment field
           Product: JMeter
           Version: 3.0
          Hardware: All
                OS: All
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: Main
          Assignee: issues@jmeter.apache.org
          Reporter: support@ubikloadpack.com

Currently comment field are useless during the run of a test plan but very
useful to allow readability and maintainability of scripts.

It would be useful to nullify this field so that it does not occupy useless
memory during the load test and avoid its transfer on the wire for distributed
tests.

As it is possible that some users have used this field for something in their
test (which would be a bad practice), we could introduce a property to allow
cancelling this behaviour if really needed.

Thoughts ?
Thanks
@ubikloadpack Team

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

[Bug 60017] Performance optimisation : Nullify comment field

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

--- Comment #5 from Sebb <se...@apache.org> ---
Thanks for the patch, but it not complete.
Also there is not yet agreement that it should be applied.

For the record:

The patch does not address:
- making the behaviour optional
- documentation of the change
- unit tests

Furthermore, the change to AbstractTestElement affects all properties, not just
comments, so may cause problems elsewhere. This could be avoided by changing
the PreCompiler to drop the comment property entirely instead of setting the
content to null. That would save more space.

I am still -1 on applying this change

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

[Bug 60017] Performance optimisation : Nullify comment field

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

UbikLoadPack support <su...@ubikloadpack.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |PatchAvailable

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

[Bug 60017] Performance optimisation : Nullify comment field

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

--- Comment #3 from Sebb <se...@apache.org> ---
Many test plans will only have a few comments, in which case the change will
not help much.

I don't think it's worth the risk.

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

[Bug 60017] Performance optimisation : Nullify comment field

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

--- Comment #4 from UbikLoadPack support <su...@ubikloadpack.com> ---
Created attachment 34166
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34166&action=edit
Patch implementing the enhancement

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

[Bug 60017] Performance optimisation : Nullify comment field

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

--- Comment #1 from Sebb <se...@apache.org> ---
Is the memory taken by comments really that much?

I don't think it's worth the risk of affecting existing test plans.
Also it will be more work for some users who have to enable/disable the option.

Unless it can be shown to have a significant effect on memory usage, I am -1.

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

[Bug 60017] Performance optimisation : Nullify comment field

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

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

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

--- Comment #2 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
(In reply to Sebb from comment #1)
> Is the memory taken by comments really that much?
> 
> I don't think it's worth the risk of affecting existing test plans.
> Also it will be more work for some users who have to enable/disable the
> option.
> 
> Unless it can be shown to have a significant effect on memory usage, I am -1.

I made a test, you multiply number of thread  * number of element * comment
size.
It is possible that the space is highly reduced by using
-XX:+UseStringDeduplication which AFAIK is not enable by default. 

Anyway, shouldn't we gain whatever we can in memory footprint ot Test plans ? 

Regarding impacted test plan:
- Either we add a property to revert behaviour (but as you wrote, it's more
code to maintain ...)
- My opinion is that why should we take into account hacks and bad practices ?
We just add a breaking change in release notes and that's it

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

[Bug 60017] Performance optimisation : Nullify comment field

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

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

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

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