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
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.
(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
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.
Created attachment 34166 [details] Patch implementing the enhancement
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
This issue has been migrated to GitHub: https://github.com/apache/jmeter/issues/4068