We heavily use BeanShell pre-, post- processor and function components. We discovered that after running for 10 minutes, we get OutOfMemoryExceptions, consistenly. We made some test with simplified scripts and we have te same - even simple BeanShell scripts cause leaks. It turns out that this problem actually originates in the BeanShell interpreter itself. The only workaround I have found constitutes of throwing out the whole interpreter object afer an eval. Looking at the sources, at the moment JMeter uses one isntance per component, per test run. To see whether the workaround can work for JMeter, we have patched the BeanShell components to instantiate a interpreters any time they need to evaluate a script. This helped and now we have bounded memory use. We probably have a performance loss, but so far this has posed no problem. Since I do not see BeanShell devs fixing their leak, perhaps JMeter should integrate BeanShell using the workaround, to keep JMeter actually usable with this scripting? Cheers, Nik
Could you attach the patch? (In reply to comment #0) > We heavily use BeanShell pre-, post- processor and function components. > > We discovered that after running for 10 minutes, we get OutOfMemoryExceptions, > consistenly. > > We made some test with simplified scripts and we have te same - even simple > BeanShell scripts cause leaks. > > It turns out that this problem actually originates in the BeanShell interpreter > itself. The only workaround I have found constitutes of throwing out the whole > interpreter object afer an eval. > > Looking at the sources, at the moment JMeter uses one isntance per component, > per test run. > > To see whether the workaround can work for JMeter, we have patched the > BeanShell components to instantiate a interpreters any time they need to > evaluate a script. This helped and now we have bounded memory use. We probably > have a performance loss, but so far this has posed no problem. > > Since I do not see BeanShell devs fixing their leak, perhaps JMeter should > integrate BeanShell using the workaround, to keep JMeter actually usable with > this scripting? > > Cheers, > Nik
Created attachment 19070 [details] example of patching a component that uses BeanShell This file exemplifies how I patched some of the components. I did not patch all of them, only pre-, post-, and function. I also changed the BeanShellInterpreter.java (see separate attachment). Perhaps one can contain all necessary changes to that file - I did not have time to do this.
Created attachment 19071 [details] - the interpreter patched Clearing the name space seems required as BeanShell has some static structure in there.
Created attachment 19281 [details] Patch to resolve memory leak issue Minimal diff to fix the BeanShell memory leak. The patch changes the BeanShellInterpreter to create an instance of the bsh.Interpreter class each time eval or source is called. Variables created in the Interpreter are loaded into a Map after each evaluation and loaded back into the new Interpreter before each evaluation to keep the same state between runs.
I've just done an experiment with a very simple script: Thread.sleep(200); return "1"; When this is run from the script pane, memory usage does climb as the test continues. However, when the script is executed from a file, memory usage sees to increase very slowly. Perhaps you can try this as a work-round.
I'm in the process of upgrading to jMeter 2.3.1 from 2.2 and re-tested our script on a vanilla 2.3.1 instance to see if the memory issue still exists. It does and in 684 samples from a single thread the BSH Listener was close to causing an out-of-memory error. I was monitoring the script execution using JConsole and have screen shots of the overall heap usage and old-gen usage after stopping the test. I also used jmap to get historgrams of the live object count/size immediately before and after stopping the test. These files along with the jmx script and bsh script are available here: https://mywebspace.wisc.edu/dalquist/web/jMeterMemoryLeak/ Note that the configuration files that specify the hosts and users used in the test are not included so the script is not runnable as is. We'll be doing a vendor branch import of 2.3.1 later this week and migrating our BSH component fix, which is attached to this issue as "Patch to resolve memory leak issue", to the 2.3.1 code and I'll post an updated patch if needed.
I would be interested in seeing a patch against 2.3.1. I've just had another look at the 2006-12-18 patch, but it does not seem to be possible to apply it to BeanShellInterpreter, even if I revert the source to revision 488388. Most of the patch ends up being rejected. Examining the patch manually is also really difficult, as there are a lot of renames and method signature changes. If you do provide another patch, it would be very helpful if it could be just the minimum needed. I can then look at incorporating it into the next release. In the meantime, could you perhaps attach the full source for your copy of BeanShellInterpreter.java? I might be able to make some headway with that.
I'm working on applying the patch to 2.3.1 now and should have a patch by the end of the week. The essence of what the previous patch did was move the bshClass.newInstance(); call from the BeanShellInterpreter constructor to the bshInvoke method so a new interpreter instance is created each time to release the handle on the previously parsed script objects. The downside of this approach is the extra object creation and the inability to pass data between BSH runs other than through jMeter properties. As far as I can tell from the BSH docs there is no way to give a bsh.Interpreter a script and just a have it run that same script over and over. I was thinking that this time around I could look at adding an option to the UI to either use a persistent Interpreter (realizing it leaks memory) or a per-call Interpreter which won't leak memory.
(In reply to comment #8) > I'm working on applying the patch to 2.3.1 now and should have a patch by the > end of the week. Thanks. > The essence of what the previous patch did was move the bshClass.newInstance(); > call from the BeanShellInterpreter constructor to the bshInvoke method so a new > interpreter instance is created each time to release the handle on the > previously parsed script objects. > > The downside of this approach is the extra object creation and the inability to pass data between BSH runs other than through jMeter properties. Yes; that's why I've been wary of adding it. > As far as I > can tell from the BSH docs there is no way to give a bsh.Interpreter a script > and just a have it run that same script over and over. You can define methods in a startup file, and call that from the screen or another file. This reduces the new objects created. > I was thinking that this time around I could look at adding an option to the UI > to either use a persistent Interpreter (realizing it leaks memory) or a > per-call Interpreter which won't leak memory. > It could be UI or a property.
Created attachment 21783 [details] Add option to reset bsh.Interpreter on each iteration This patch is a slightly different approach to the problem than the patch for 2.2. In BeanShellInterpreter the code that creates the new bsh.Interpreter instance and that calls init(String, Logger) has been moved into a public reset() method. This method is called by both constructors to retain the existing behavior but gives clients of the class a way to re-created and re-init the Interpreter. All of the jMeter components that use the BeanShellInterpreter have been updated to provide a per-component configuration option to reset the interpreter on each iteration, this defaults to false. All of the BeanShellBeanInfoSupport based UIs use a select box for the true/false selection, the assertion and sampler GUIs use a checkbox. It seems like a checkbox would be preferable for the BeanShellBeanInfoSupport based UIs but I couldn't figure out an easy way to add a JCheckBox to them. I did not update the BeanShell function as I'm not sure if that needs the option as well, if you think it does let me know and I can add an option for a reset call there as well. The patch should be the absolute minimal diff based on 2.3.1, please let me know if you have problems applying it or questions about the patch. Thank you, Eric
Thanks very much. The patch worked OK with the current code (after 2.3.1) with only a minor tweak. I had to change it slightly, because public functions cannot be called from constructors. In the end, I merged the init() and reset() methods, as there seemed no reason to keep them separate. I've committed the changes in: http://svn.apache.org/viewvc?view=rev&revision=644823 and http://svn.apache.org/viewvc?rev=644831&view=rev If you want to test them, they are in the jakarta-jmeter-r644833 nightly build (and later ones). A previous patch also saved and restored variables. Would this still be useful, perhaps as a 3rd option? I.e reuse / reset / reset,keeping variables I don't think the BeanShell function needs a reset option. [As to TestBean checkboxes - they have not been implemented.]
Thanks for applying the patch, combining init and reset is a good move I just wanted to keep the diffs as small as possible. As for copying the data between runs, it shouldn't be too hard logically bug doing it all via reflection may be painful as there are a few other BSH objects that look like they need to be referenced to make it happen. If it is an important feature let me know and I can see about implementing it.
If BeanShell scripts need to save data between invocations, as a work-round, they can use JMeter variables. Closing the bug. If some form of automatic variable saving is needed, that would be best dealt with in an enhancement request.
What about using Beanshell as a function, via __BeanShell? I'm seeing a massive drop in performance in my test plan after 5-10minutes, and I use beanshell functions inside my sampler (typically via a source() call). I believe the root cause is the same. From inspecting the code and the patch it doesn't appear that there is a way to reset the interpreter on each function call.
Using beanshell.function.init instead of using source() should reduce memory usage. You can define various functions in the same file. If you wish to add a reset option to the __BeanShell function, please open a new Buzilla issue.
This issue has been migrated to GitHub: https://github.com/apache/jmeter/issues/1818