Bug 60687 - Make GUI more responsive when it gets a lot of events
Summary: Make GUI more responsive when it gets a lot of events
Status: CLOSED FIXED
Alias: None
Product: JMeter - Now in Github
Classification: Unclassified
Component: Main (show other bugs)
Version: 3.1
Hardware: All All
: P2 enhancement (vote)
Target Milestone: ---
Assignee: JMeter issues mailing list
URL:
Keywords:
: 59258 (view as bug list)
Depends on:
Blocks: 60961 61002
  Show dependency tree
 
Reported: 2017-02-03 18:21 UTC by Felix Schumacher
Modified: 2017-09-07 07:21 UTC (History)
1 user (show)



Attachments
Use a bounded fifo buffer for the log events and update LoggerPanel periodically instead of directly (4.46 KB, patch)
2017-02-03 18:21 UTC, Felix Schumacher
Details | Diff
Use a bounded fifo buffer for the log events and update LoggerPanel periodically instead of directly (4.60 KB, patch)
2017-02-04 15:24 UTC, Felix Schumacher
Details | Diff
In SummaryReport: Move calculations out of AWT thread and refresh GUI only periodically. (3.96 KB, patch)
2017-02-04 15:27 UTC, Felix Schumacher
Details | Diff
Guard clearance of buffer and only set text in logger panel in one way. (970 bytes, patch)
2017-02-04 15:37 UTC, Felix Schumacher
Details | Diff
Update View Results Tree only periodically (7.71 KB, patch)
2017-02-08 11:45 UTC, Felix Schumacher
Details | Diff
Simple test case to torture the LoggerPanel and View Results Tree (4.14 KB, application/xml)
2017-02-08 11:46 UTC, Felix Schumacher
Details
Use a bounded fifo buffer for the log events and update LoggerPanel periodically instead of directly (4.81 KB, patch)
2017-02-08 11:48 UTC, Felix Schumacher
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Felix Schumacher 2017-02-03 18:21:28 UTC
Created attachment 34714 [details]
Use a bounded fifo buffer for the log events and update LoggerPanel periodically instead of directly

The LoggerPanel gets every LogEvent added in the AWT thread. This can lead to contention of the AWT event queue, which renders the UI unusable.

The attached patch passes the LogEvents into a bounded fifo buffer, that is used periodically to update the LoggerPanel. This takes a lot of stress from the AWT event queue.
Comment 1 Philippe Mouawad 2017-02-03 22:33:20 UTC
Comment on attachment 34714 [details]
Use a bounded fifo buffer for the log events and update LoggerPanel periodically instead of directly

It looks good to me.
Thanks
Comment 2 Felix Schumacher 2017-02-04 15:24:55 UTC
Created attachment 34715 [details]
Use a bounded fifo buffer for the log events and update LoggerPanel periodically instead of directly

Minor cleanups compared to the last patch.
Comment 3 Felix Schumacher 2017-02-04 15:27:47 UTC
Created attachment 34716 [details]
In SummaryReport: Move calculations out of AWT thread and refresh GUI only periodically.

Basically the same idea as in the other thread. Move the data manipulation out of the AWT thread and check for changes periodically.

A simple test case with 500 threads and a single JSR-223 groovy sampler (that does nothing) and a SummaryReport showed an increase of requests from 15.000 req/s to 19.000 req/s.
Comment 4 Philippe Mouawad 2017-02-04 15:36:28 UTC
(In reply to Felix Schumacher from comment #3)
> Created attachment 34716 [details]
> In SummaryReport: Move calculations out of AWT thread and refresh GUI only
> periodically.
> 
> Basically the same idea as in the other thread. Move the data manipulation
> out of the AWT thread and check for changes periodically.
> 
> A simple test case with 500 threads and a single JSR-223 groovy sampler
> (that does nothing) and a SummaryReport showed an increase of requests from
> 15.000 req/s to 19.000 req/s.
Great !
Maybe rename bugzilla.
Comment 5 Felix Schumacher 2017-02-04 15:37:08 UTC
Created attachment 34717 [details]
Guard clearance of buffer and only set text in logger panel in one way.
Comment 6 UbikLoadPack support 2017-02-06 08:00:38 UTC
+1 for patches.
Note that View Results Tree is probably the biggest danger for JMeter stability in GUI mode.

Regards
Comment 7 Vladimir Sitnikov 2017-02-06 08:37:47 UTC
Felix, would you please share the JMX you use to benchmark the changes?

It might be even better to include the JMX right into the patch itself.
Comment 8 Felix Schumacher 2017-02-08 11:45:48 UTC
Created attachment 34736 [details]
Update View Results Tree only periodically

In order to keep the GUI responsive, update the View Results Tree periodically, only. As a plus, keep only the last 'x' samples to protect ourselve against OOM exceptions.
Comment 9 Felix Schumacher 2017-02-08 11:46:46 UTC
Created attachment 34737 [details]
Simple test case to torture the LoggerPanel and View Results Tree
Comment 10 Felix Schumacher 2017-02-08 11:48:59 UTC
Created attachment 34738 [details]
Use a bounded fifo buffer for the log events and update LoggerPanel periodically instead of directly
Comment 11 Felix Schumacher 2017-02-08 19:01:53 UTC
Date: Wed Feb  8 18:47:24 2017
New Revision: 1782227

URL: http://svn.apache.org/viewvc?rev=1782227&view=rev
Log:
Keep GUI responsive when many events are processed by View Results Tree, Summary Table and Log Panel.
This is part one of three with the changes to Log Panel.

Bugzilla Id: 60687

Modified:
    jmeter/trunk/bin/jmeter.properties
    jmeter/trunk/src/core/org/apache/jmeter/gui/LoggerPanel.java
    jmeter/trunk/xdocs/changes.xml
    jmeter/trunk/xdocs/usermanual/properties_reference.xml

Date: Wed Feb  8 18:49:05 2017
New Revision: 1782228

URL: http://svn.apache.org/viewvc?rev=1782228&view=rev
Log:
Keep GUI responsive when many events are processed by View Results Tree, Summary Report and Log Panel.
This is part two of three with the changes to Summary Report.

Bugzilla Id: 60687


Modified:
    jmeter/trunk/src/components/org/apache/jmeter/visualizers/SummaryReport.java


Date: Wed Feb  8 19:00:26 2017
New Revision: 1782229

URL: http://svn.apache.org/viewvc?rev=1782229&view=rev
Log:
Keep GUI responsive when many events are processed by View Results Tree, Summary Report and Log Panel.
This is part three of three with the changes to View Results Tree.

Bugzilla Id: 60687


Modified:
    jmeter/trunk/bin/jmeter.properties
    jmeter/trunk/src/components/org/apache/jmeter/visualizers/ViewResultsFullVisualizer.java
    jmeter/trunk/xdocs/usermanual/properties_reference.xml
Comment 12 Felix Schumacher 2017-02-10 19:47:21 UTC
Date: Fri Feb 10 19:46:02 2017
New Revision: 1782509

URL: http://svn.apache.org/viewvc?rev=1782509&view=rev
Log:
Simplify implementation.

Bugzilla Id: 60687

Modified:
    jmeter/trunk/src/core/org/apache/jmeter/gui/LoggerPanel.java
Comment 13 Felix Schumacher 2017-02-10 20:05:30 UTC
Date: Fri Feb 10 20:04:58 2017
New Revision: 1782515

URL: http://svn.apache.org/viewvc?rev=1782515&view=rev
Log:
Simplify code and implement the option to keep all results in the view (even if it will consume a lot of memory).

Bugzilla Id: 60687

Modified:
    jmeter/trunk/bin/jmeter.properties
    jmeter/trunk/src/components/org/apache/jmeter/visualizers/ViewResultsFullVisualizer.java
    jmeter/trunk/xdocs/usermanual/properties_reference.xml
Comment 14 Vladimir Sitnikov 2017-02-12 17:37:58 UTC
I think SummaryReport uses unsafe approach of updating ObjectTableModel in the sampling threads.

Does it make sense to decouple stats collection from stat visualization?

If I get it right, we should avoid using swing.* as a stat collection/aggregation facility.
Otherwise we have to use Swing.invokeLater on the hot path.
Comment 15 Felix Schumacher 2017-02-13 10:12:08 UTC
(In reply to Vladimir Sitnikov from comment #14)
> I think SummaryReport uses unsafe approach of updating ObjectTableModel in
> the sampling threads.

Could you be a bit more specific here? I think the only problem we have is visibility, as the swing thread locks on lock and the other threads on lock and rows. So I think of it as a kind of eventual visibility.

> 
> Does it make sense to decouple stats collection from stat visualization?

That was my first thought, but it would introduce another thread. We could drop the row locks, though.

> 
> If I get it right, we should avoid using swing.* as a stat
> collection/aggregation facility.
> Otherwise we have to use Swing.invokeLater on the hot path.

Right. Get swing out of the path and be happy.
Comment 16 Vladimir Sitnikov 2017-02-13 12:58:54 UTC
Felix> Could you be a bit more specific here?

For instance,
1) Things like "model.insertRow" do require to be executed on the AWT thread.
I'm not sure how hard it could break, however accessing UI thread on the hot path is bad.

2) Table rendering code (that is run on the UI thread) accesses computed values without any synchronization. This might result in word tearing, stale values, unexpected division by zero, etc.


Felix> Right. Get swing out of the path and be happy.

This kind of change would likely to heal 1 above, however it might look like a "complete rewrite of the relevant listener code".

Even if we move swing calls out of hot path, we would still need some fast way to aggregate values.
For instance, "Total" row could be computed "on UI refresh" to avoid global synchronization of all the threads.

From one point of view this might look like a over engineering, on the other hand, that might be reused for "non-gui console mode" listeners.
Comment 17 Philippe Mouawad 2017-02-13 21:32:17 UTC
Hello,

There might be another place where GUI is impacted, it's in:
- MainFrame#ErrorsAndFatalsCounterLogTarget
- And less importantly MainFrame#updateCounts

Regards
Comment 18 Philippe Mouawad 2017-02-15 20:52:51 UTC
(In reply to Vladimir Sitnikov from comment #16)
> Felix> Could you be a bit more specific here?
> 
> For instance,
> 1) Things like "model.insertRow" do require to be executed on the AWT thread.

Indeed, good catch. Particularly knowing that insertRow calls fireTableRowsInserted.

Can't we just collect rows in a list and insert in model from Timer ? But it will take more time.

> I'm not sure how hard it could break, however accessing UI thread on the hot
> path is bad.
> 
> 2) Table rendering code (that is run on the UI thread) accesses computed
> values without any synchronization. This might result in word tearing, stale
> values, unexpected division by zero, etc.
> 
> 
> Felix> Right. Get swing out of the path and be happy.
> 
> This kind of change would likely to heal 1 above, however it might look like
> a "complete rewrite of the relevant listener code".
> 
> Even if we move swing calls out of hot path, we would still need some fast
> way to aggregate values.
> For instance, "Total" row could be computed "on UI refresh" to avoid global
> synchronization of all the threads.
> 
> From one point of view this might look like a over engineering, on the other
> hand, that might be reused for "non-gui console mode" listeners.

So this means we have a blocker for 3.2 now.
Comment 19 Philippe Mouawad 2017-02-15 21:50:19 UTC
With below improvement, passed from 3566 samples / sec to 4100/sec



Author: pmouawad
Date: Wed Feb 15 21:45:23 2017
New Revision: 1783153

URL: http://svn.apache.org/viewvc?rev=1783153&view=rev
Log:
Bug 60687 - Make GUI more responsive when it gets a lot of events
Improve errrors indicator management
Bugzilla Id: 60687

Modified:
    jmeter/trunk/src/core/org/apache/jmeter/gui/MainFrame.java
Comment 20 Vladimir Sitnikov 2017-02-16 12:06:08 UTC
Philippe> With below improvement

Should the timer be stopped at some point like computeTestDurationTimer is stopped in testEnded ?
Comment 21 Philippe Mouawad 2017-02-16 12:17:27 UTC
(In reply to Vladimir Sitnikov from comment #20)
> Philippe> With below improvement
> 
> Should the timer be stopped at some point like computeTestDurationTimer is
> stopped in testEnded ?

I don't think so as it's tightly related to Jmeter gui so it looked ok to me to not stop it and just wait for exit.


but if you think we should feel free to apend my code. thx
Comment 22 Felix Schumacher 2017-02-16 18:51:56 UTC
@Vladimir, do you think this solution is better suited?

Date: Thu Feb 16 18:41:11 2017
New Revision: 1783270

URL: http://svn.apache.org/viewvc?rev=1783270&view=rev
Log:
Keep GUI responsive when many events are processed by View Results Tree, Summary Report and Log Panel.
Modify the model only, when we are in the AWT thread. Use more features of ConcurrentHashMap.

Bugzilla Id: 60687


Modified:
    jmeter/trunk/src/components/org/apache/jmeter/visualizers/SummaryReport.java
Comment 23 Felix Schumacher 2017-02-18 14:09:14 UTC
Date: Sat Feb 18 14:06:36 2017
New Revision: 1783556

URL: http://svn.apache.org/viewvc?rev=1783556&view=rev
Log:
Make GUI more responsive when it gets a lot of events.
Convert Aggregate Graph to use a queue, when new rows are to be inserted.
Update the model (or at least notify the gui) only periodically.
That way we get the update of the model out of the busy path of the swing
event loop.

Bugzilla Id: 60687

Modified:
    jmeter/trunk/src/components/org/apache/jmeter/visualizers/StatGraphVisualizer.java
Comment 24 Felix Schumacher 2017-02-18 14:32:34 UTC
Date: Sat Feb 18 14:31:45 2017
New Revision: 1783558

URL: http://svn.apache.org/viewvc?rev=1783558&view=rev
Log:
Make GUI more responsive when it gets a lot of events.
Convert Aggregate Report to use a queue, when new rows are to be inserted.
Update the model (or at least notify the gui) only periodically.
That way we get the update of the model out of the busy path of the swing
event loop.

Bugzilla Id: 60687

Modified:
    jmeter/trunk/src/components/org/apache/jmeter/visualizers/StatVisualizer.java
Comment 25 Felix Schumacher 2017-02-18 19:23:21 UTC
Date: Sat Feb 18 18:58:08 2017
New Revision: 1783577

URL: http://svn.apache.org/viewvc?rev=1783577&view=rev
Log:
Recurse into add method before changing into the runSafe method.

Bugzilla Id: 60687

Modified:
    jmeter/trunk/src/components/org/apache/jmeter/visualizers/TableVisualizer.java

Date: Sat Feb 18 19:22:40 2017
New Revision: 1783579

URL: http://svn.apache.org/viewvc?rev=1783579&view=rev
Log:
Make GUI more responsive when it gets a lot of events.
Convert Table Visualizer to use a queue, when new rows are to be inserted.
Update the model (or at least notify the gui) only periodically.
That way we get the update of the model out of the busy path of the swing
event loop. Note, that this will not save us from OOM exceptions.

Bugzilla Id: 60687


Modified:
    jmeter/trunk/src/components/org/apache/jmeter/visualizers/TableVisualizer.java
Comment 26 Felix Schumacher 2017-02-18 21:11:11 UTC
Date: Sat Feb 18 21:10:09 2017
New Revision: 1783590

URL: http://svn.apache.org/viewvc?rev=1783590&view=rev
Log:
Make the interval period configurable by one property for all listeners.

Bugzilla Id: 60687

Modified:
    jmeter/trunk/bin/jmeter.properties
    jmeter/trunk/src/components/org/apache/jmeter/visualizers/GraphVisualizer.java
    jmeter/trunk/src/components/org/apache/jmeter/visualizers/StatGraphVisualizer.java
    jmeter/trunk/src/components/org/apache/jmeter/visualizers/StatVisualizer.java
    jmeter/trunk/src/components/org/apache/jmeter/visualizers/TableVisualizer.java
    jmeter/trunk/src/components/org/apache/jmeter/visualizers/ViewResultsFullVisualizer.java
    jmeter/trunk/src/core/org/apache/jmeter/gui/LoggerPanel.java
    jmeter/trunk/xdocs/usermanual/properties_reference.xml
Comment 27 Philippe Mouawad 2017-03-08 21:01:31 UTC
*** Bug 59258 has been marked as a duplicate of this bug. ***
Comment 28 Felix Schumacher 2017-03-10 18:59:17 UTC
Date: Fri Mar 10 18:58:51 2017
New Revision: 1786417

URL: http://svn.apache.org/viewvc?rev=1786417&view=rev
Log:
Use jmeter.gui.refresh_period to configure period of refresh for Summary Report.

Bugzilla Id: 60687

Modified:
    jmeter/trunk/src/components/org/apache/jmeter/visualizers/SummaryReport.java
Comment 29 andreaslind01 2017-04-06 13:59:42 UTC
(JMeter 3.2 rc2)
The latest changes in the ViewResultsTree listener lead to selections getting lost as soon as new items show up in the results tree.
Comment 30 Philippe Mouawad 2017-09-02 19:22:39 UTC
Fixed within Bug 60961
Comment 31 The ASF infrastructure team 2022-09-24 20:38:07 UTC
This issue has been migrated to GitHub: https://github.com/apache/jmeter/issues/4264