For a test plan with 2 samplers Sampler A Sampler B Switching from A to B will trigger too many gui (JMeterGUIComponent) updates / configuration Precisely : 1 JMeterGUIComponent#modifyTestElement for A 3 JMeterGUIComponent#modifyTestElement for B 4 JMeterGUIComponent#configure for B 3 JMeterGUIComponent#clearGui for B The root cause is GuiPackage #getCurrentGui being call 3 times EditCommand : 2 times MenuFactory#addFileMenu : 1 For components which do costly computation in JMeterGUIComponent#configure (eg : WebServiceSamplerGui) this bug introduce a lag the jmeter gui usage. If the component displays a popup in JMeterGUIComponent#configure, it will be displayed 3 times… I think we should review the way GuiPackage #getCurrentGui is used and at least rename it because this method is doing much more than getting the current gui. The first patch does not correct the root cause but save 1 invocation.
Created attachment 27501 [details] Save 1 invocation of GuiPackage #getCurrentGui Save 1 invocation of GuiPackage#getCurrentGui Benoit Wiart Ubik Ingénierie www.ubik-ingenierie.com
GuiPackage#updateCurrentGui is almost identical to GuiPackage#getCurrentGui except it does not call clearGui What could be done : change updateCurrentGui to match getCurrentGui (add clearGui and try-catch) change getCurrentGui to simply return the current gui (nothing more) Some call sites of getCurrentGui rely on the fact it will "save" the current gui eg GuiPackage#localeChanged or JMeterTreeModel#addComponent they must be updated to call updateCurrentGui I have a working prototype which do not regress on bugs (43122 30120, 41078, 39427, 50221, 42346) or revisions (325250, 324131) Dear Jmeter maintainers let me know what you think about that... Benoit Wiart
(In reply to comment #2) > GuiPackage#updateCurrentGui is almost identical to GuiPackage#getCurrentGui > except it does not call clearGui > > What could be done : > change updateCurrentGui to match getCurrentGui (add clearGui and try-catch) > change getCurrentGui to simply return the current gui (nothing more) > > Some call sites of getCurrentGui rely on the fact it will "save" the current > gui > eg GuiPackage#localeChanged or JMeterTreeModel#addComponent > they must be updated to call updateCurrentGui Please raise a separate issue for this.
(In reply to comment #3) > (In reply to comment #2) > > GuiPackage#updateCurrentGui is almost identical to GuiPackage#getCurrentGui > > except it does not call clearGui > > > > What could be done : > > change updateCurrentGui to match getCurrentGui (add clearGui and try-catch) > > change getCurrentGui to simply return the current gui (nothing more) > > > > Some call sites of getCurrentGui rely on the fact it will "save" the current > > gui > > eg GuiPackage#localeChanged or JMeterTreeModel#addComponent > > they must be updated to call updateCurrentGui > > Please raise a separate issue for this. Hi Sebb My comment 2 seems out of context... BUT the change on updateCurrentGui and getCurrentGui is a way to correct this issue by removing the 'extra work' done in getCurrentGui. Unless you really want to, I don't think it should be part of another bug
Seems to me that a better solution to the WebServiceSamplerGui.configure issue would be to move the expensive computation elsewhere. The configure() method was intended for copying the TestElement fields to the GUI, not for doing GUI display manipulation.
(In reply to comment #5) > Seems to me that a better solution to the WebServiceSamplerGui.configure issue > would be to move the expensive computation elsewhere. > > The configure() method was intended for copying the TestElement fields to the > GUI, not for doing GUI display manipulation. I agree, we could also add some caching in wdslhelper But we should also correct the multiples calls to JMeterGUIComponent#modifyTestElement, JMeterGUIComponent#configure, JMeterGUIComponent#clearGui done through getCurrentGui Tell me if you don't want to impact getCurrentGui / updateCurrentGui : then you can only apply the first patch. Otherwise I will create a patch from my prototype. I'm waiting for your 'GO / NOGO'
(In reply to comment #1) > Created attachment 27501 [details] > Save 1 invocation of GuiPackage #getCurrentGui > > Save 1 invocation of GuiPackage#getCurrentGui > Thanks, that should help. URL: http://svn.apache.org/viewvc?rev=1171938&view=rev Log: Bug 51822 - (part 1) save 1 invocation of GuiPackage#getCurrentGui Modified: jakarta/jmeter/trunk/src/core/org/apache/jmeter/gui/action/EditCommand.java jakarta/jmeter/trunk/xdocs/changes.xml
Hello Benoit, Can you provide the patch you describe in comment 1 and 6 ? I change this to an enhancement. Regards Philippe
This issue has been migrated to GitHub: https://github.com/apache/jmeter/issues/2536