-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make refresh rate of DeferredUpdateManager & TargetingTool configurable #550
Conversation
A follow-up to #494. After thinking about this topic for a while, I also came to the conclusion that dealing with system properties and alike is way too complicated, so I opted to a simple variable one can set. |
I had a look at the Shouldn't all refresh requests that are arriving during during the refresh rate be discarded? |
That should already be the case right now: gef-classic/org.eclipse.draw2d/src/org/eclipse/draw2d/DeferredUpdateManager.java Lines 249 to 254 in 0214a12
The manager queues the update and then ignores everything that is submitted until the update is executed. This change simply delays the execution by a fixed amount of time. In both cases, you will always have only one request scheduled at a time. But with the refresh rate, it'll be less requests per minute that are executed. |
Thanks for the explanation. Yes that makes sense. I just tested it with larger diagrams (a few thousand figures) in 4diac IDE. I gave the UpdateManager of the editor a refresh rate of 20ms and the outline (which was up to now the bottleneck) 100ms. I didn't see any drawing artifacts, didn't notice any lag in refresh, and what it is most important it felt smother then before. However that is a bit subjective and so far I didn't find a good way to measure it. @BauePhil you could give this patch a try to test if you get a better performance for your large Zest diagrams. |
I don't think there is, as I believe this to highly depend on the complexity of the model. For the targeting tool, 1ms seems to have been a good value, but even that is hard to check without a big sample size. |
This adds a new setRefreshRate() method to the Thumbnail, DeferredUpdateManager and TargetingTool class which defines how often UI events are executed. By default, those events are executed as fast possible, which may lead to to a very high CPU cost with very little gain. By setting a high rate, users may artificially throttle the speed at which those operations are performed, at the cost of responsiveness. Relates to: eclipse#430 eclipse#492
In #430 you said:
Is this approach similar to what you had in mind? Reducing the number of executed paint requests should behave similar to repainting the same image over multiple calls. The "buffer" would then be whatever has been stored in the native widget and not be a separate Image. Ideally, I'd like to finish this PR up for the next release, so that you and others can give it a go. |
@ptziegler I gave it another try today. And I strongly believe that especially opening very large diagrams feels now much more responsive. So from my side I would merge it. |
I haven't got the time to test this issue, but looking at the code I am not sure how it buffers the updates? As far as can I see, Display#timerExec is always called with a new object (only in Thumbnail the same "this" reference is being used) so this only delays the execution of the Runnable. I would expect the same instance of the object to be used with timerExec to get the "buffering" effect, like it's done in Thumbnail. Am I wrong? |
For e.g. the DeferredUpdateManager, most of the magic is done by the gef-classic/org.eclipse.draw2d/src/org/eclipse/draw2d/DeferredUpdateManager.java Lines 250 to 255 in 5c33111
|
This adds a new setRefreshRate() for both classes, which defines how often paint requests are executed/the auto-exposer helper is evaluated.
Both tasks are currently executed as fast possible, which may lead to to a very high CPU cost with very little gain. By setting a high rate, users may artificially throttle the speed at which those operations are performed, at the cost of responsiveness.
Relates to:
#430
#492