Community
Participate
Working Groups
Created attachment 112710 [details] Patch with proposed fix. The IElementLabelProvider.update() method takes an array of label updates as an argument, however the TreeModelLabelProvider implementation, which calls this method always passes a single update to the element label providers. We can easily optimize the issuing of these updates by not calling the element label provider immediately, but rather waiting for additional update requests. I don't think we really need to use a timer as in the ModelContentProvider, because typically the calls to label update are made within a tight loop from ChildrenUpdate, or by the SWT control in a quick succession. A simple alternative to using a timer is to simply submit a runnable to the display , which will be executed after all the other pending runnables, and to issue the updates from that runnable. The attached patch implements this mechanism.
Created attachment 115956 [details] patch This patch uses the same approach, but uses a UIJob rather than using the async exec function directly. This allows for better handling of shutdown while the job has been scheduled but not yet run. As well, it allows the pending job to be cancelled when the label provided is disposed.
Pawel, please review and apply if you are happy with the changes.
I added a cancellation of the pending job when a new update job is created and additional cleanup on dispose. I agree that using a UI job is better at least for sake of consistency, but I also think that it's worth reevaluating whether using UIJob is really appropriate in the flexible hierarchy viewer in general. I see two downsides to it: 1) The flexible hierarchy viewer cannot be used without the runtime plugin. When working on the the virtual flexible hierarchy viewer (bug 242489) I got rid of the UI jobs so that I could run pure java unit tests on the viewer. 2) They are much less efficient than using asyncExec directly. Each UI job is first run in a worker thread before being scheduled with the display thread. When the async calls are being used very liberally (as they are here, where each label update creates a new job), this overhead can be rather significant. We had a similar design issue in DSF, where the executor for the UI thread would always first execute a runnable in a backgroun thread just to call Display.syncExec(). After we got rid of this and called Display.asyncExec() directly, we saw a visible UI performance improvment. Although in DSF we submit runnables an order of magnitude more often than in the flexible hierarchy viewer. Anyway, I committed the fix. Please verify.
Verified.
