eclipse
Make refresh rate of DeferredUpdateManager & TargetingTool configurable
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: https://github.com/eclipse/gef-classic/issues/430 https://github.com/eclipse/gef-classic/issues/492
A follow-up to https://github.com/eclipse/gef-classic/pull/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 timerExec and I don't think your patch does what you want it to do. If I understand it correctly you just delay every refresh request by the refresh rate but you are not reducing the number of refresh request, or?
Shouldn't all refresh requests that are arriving during during the refresh rate be discarded?
Shouldn't all refresh requests that are arriving during during the refresh rate be discarded?
That should already be the case right now:
https://github.com/eclipse/gef-classic/blob/0214a1215efebb55db50e0ecfe074a0dcd907d21/org.eclipse.draw2d/src/org/eclipse/draw2d/DeferredUpdateManager.java#L249-L254
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.
However that is a bit subjective and so far I didn't find a good way to measure it.
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. The sweet spot for their project is something people have to figure out on their own, I'm afraid.
@wiresketch
In #430 you said:
What I had in mind is reusing the same image for multiple paint calls without redrawing the buffered image each time.
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.
@wiresketch
In #430 you said:
What I had in mind is reusing the same image for multiple paint calls without redrawing the buffered image each time.
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.
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?
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.
For e.g. the DeferredUpdateManager, most of the magic is done by the updateQueued flag. While the request is pending, all successive calls to the udpate method (e.g. during paint calls) are effectively skipped. By delaying the request execution, one can greatly reduce the number of repaints that are executed within a given amount of time.
https://github.com/eclipse/gef-classic/blob/5c33111b0df3631c02ebe9ad28aabb143b283c3e/org.eclipse.draw2d/src/org/eclipse/draw2d/DeferredUpdateManager.java#L250-L255