Community
Participate
Working Groups
See https://www.eclipse.org/lists/platform-dev/msg01749.html and https://bugs.eclipse.org/bugs/show_bug.cgi?id=538630#c5 and https://bugs.eclipse.org/bugs/show_bug.cgi?id=548891#c5 , there is a more frequent need to make some extension point evolve and allow extensions to run their job in a non-UI-thread (when they can) to make IDE more reactive. So we need a way for a piece of code to declare whether it can run in any thread or whether it must run in the UI Thread.
Current proposal would be addition of a @AnyThread annotation that could be placed on classes and that consumers could read by introspection to decide how to handle this extension. We don't have need for an opposite @UIThread annotation at the moment, as this is already the default expectation for a bunch of extension points.
New Gerrit change created: https://git.eclipse.org/r/145465
(In reply to Eclipse Genie from comment #2) > New Gerrit change created: https://git.eclipse.org/r/145465 May be we can declare thread-related annotations in some neutral place ouside the SWT? I would suggest to use the separate bundle for it. In that case it may be widely used for other scenarios.
(In reply to Alexander Fedorov from comment #3) > May be we can declare thread-related annotations in some neutral place > ouside the SWT? I would suggest to use the separate bundle for it. In that > case it may be widely used for other scenarios. While I'm aware of many cases where this would help to handle SWT work and SWT UI Thread vs other Threads, I'm not much aware of other existing cases where it's relevant. Moreover, by having it in SWT, we can make it pretty clear in the documentation what it can help with, while having something generic would lead to some more generic and less actionable documentation. Also, the general problem of AnyThread or not is more a Java one in the end. So the right place where to fix it would probably be more in the Java APIs than inside Eclipse. As you've understood, I prefer keeping focus on the problems we have in Platform that on the problems some other may have in other frameworks ;)
(In reply to Mickael Istria from comment #4) > > As you've understood, I prefer keeping focus on the problems we have in > Platform that on the problems some other may have in other frameworks ;) OK, I see your point. I used to think that you are going to process this annotation somewhere outside the SWT during the extension processing, i.e. from some place that shouldn't have direct access to SWT bundle.
SWT itself doesn't use this annotation in any way, it should be moved elsewhere, especially when it comes with heavy backward compatibility constraints. BTW, does it have to be an annotation? Wouldn't a Javadoc comment suffice? E.g. exception specifications in SWT use Javadoc. Why not add threading specification in the same vein?
(In reply to Nikita Nemkin from comment #6) > SWT itself doesn't use this annotation in any way There is a 1st time for everything ;) > it should be moved > elsewhere, especially when it comes with heavy backward compatibility > constraints. Where do you think it would be best, especially since they are purely about SWT in the end (they answer no to question «do I need to run this in the UI Thread»). > BTW, does it have to be an annotation? Wouldn't a Javadoc comment suffice? > E.g. exception specifications in SWT use Javadoc. Why not add threading > specification in the same vein? We need the annotations to be found at runtime, so it has to be runtime data (@Retention(RUNTIME)) and cannot be doc nor source/class retention. See bug marked as dependent for more contect how this could be used in JDT to make completion not block UI Thread.
@Mickael may be you can submit a gerrit with "counterpart" for this annotation that will demonstrate the best way to use it.
(In reply to Mickael Istria from comment #7) > (In reply to Nikita Nemkin from comment #6) > > SWT itself doesn't use this annotation in any way > > There is a 1st time for everything ;) The annotation you propose would be impossible to use in SWT, even in the future. SWT doesn't have any thread safe classes. What SWT does have is 1) classes with UI thread affinity (widgets), 2) classes without UI thread affinity (graphics) and 3) a few thread-safe methods. And I think JavaDoc is the best place for this info. > > it should be moved > > elsewhere, especially when it comes with heavy backward compatibility > > constraints. > > Where do you think it would be best, especially since they are purely about > SWT in the end (they answer no to question «do I need to run this in the UI > Thread»). TBH I have pretty vague idea of layers above SWT. But runtime annotation to gate thread-safe IDE plugins/services is obviously out of SWT scope. > > BTW, does it have to be an annotation? Wouldn't a Javadoc comment suffice? > > E.g. exception specifications in SWT use Javadoc. Why not add threading > > specification in the same vein? > > We need the annotations to be found at runtime, so it has to be runtime data > (@Retention(RUNTIME)) and cannot be doc nor source/class retention. > See bug marked as dependent for more contect how this could be used in JDT > to make completion not block UI Thread. I understand now. Have you considered adding new extension point for async completion handlers? Wouldn't that be cleaner than checking runtime annotations on extension point implementations?
New Gerrit change created: https://git.eclipse.org/r/145474
(In reply to Alexander Fedorov from comment #8) > @Mickael may be you can submit a gerrit with "counterpart" for this > annotation that will demonstrate the best way to use it. Good idea, here is what it would look like: https://git.eclipse.org/r/145474 (In reply to Nikita Nemkin from comment #9) > The annotation you propose would be impossible to use in SWT, even in the > future. Ok, but it's really meant to make people using SWT more successful at dealing with SWT (ie avoid InvalidThreadAccess or avoid freezing UI Thread), so while SWT itself may not use it (and it actually could use it on listeners for instance), it's still something that would benefit to the SWT ecosystem at large, so it would make sense to have it in SWT. > TBH I have pretty vague idea of layers above SWT. But runtime annotation to > gate thread-safe IDE plugins/services is obviously out of SWT scope. It's not about being thread-safe, it's about stating some piece of code doesn't require to run in UI Thread. > I understand now. Have you considered adding new extension point for async > completion handlers? Wouldn't that be cleaner than checking runtime > annotations on extension point implementations? A new extension point would be too much, but a flag on existing extension point was/is the 1st proposal. However, while working on it, I have the impression that "whether the class must run in UI Thread or not" doesn't depend on the extension but on the code itself, so I think something in-code would be far more reusable.
I agree with @Mickael we should try to get the community step by step to where we want. This approach will also simplify the maintenance over adding a new extension if i understood correctly. And also i think we should add this support for marking (annotating support) in the common place where it can be used in other places as well. As a novice eclipse developer i think this belongs in more core area. But whether it should belong to SWT or somewhere else depends on how much UI toolkits eclipse will run on. For example SWT will not be a candidate if eclipse will run on another UI Toolkit.
(In reply to Gayan Perera from comment #12) > And also i think we should add this support for marking (annotating support) > in the common place where it can be used in other places as well. As a > novice eclipse developer i think this belongs in more core area. But whether > it should belong to SWT or somewhere else depends on how much UI toolkits > eclipse will run on. For example SWT will not be a candidate if eclipse will > run on another UI Toolkit. All the examples of service providers I have in mind that suffer from issues of frozen UI Thread have SWT as backend. And the consumers also render to SWT, and moreover, a piece of code that's non-UI-Thread-able in a framework may not be non-UI-Thread-able in SWT. For example, control.getBackground() color may not require UI Thread in some other UI Frameworks like Android or JavaFX. So whether a piece of code can run in non-UI Thread or not really depends on the UI backend details, it's not something we can state as true for all UI libraries, so it has to be specific to each library.
(In reply to Mickael Istria from comment #13) > All the examples of service providers I have in mind that suffer from issues > of frozen UI Thread have SWT as backend. And the consumers also render to > SWT, and moreover, a piece of code that's non-UI-Thread-able in a framework > may not be non-UI-Thread-able in SWT. For example, control.getBackground() > color may not require UI Thread in some other UI Frameworks like Android or > JavaFX. > So whether a piece of code can run in non-UI Thread or not really depends on > the UI backend details, it's not something we can state as true for all UI > libraries, so it has to be specific to each library. JavaFX and Android UI are also single-UI-threaded. This approach is nearly universal across UI frameworks. Anyway, I'm -1 on having this in SWT. If every "useful" thing was added to SWT, we wouldn't have JFace. The rest is not my area of expertise.
> JavaFX and Android UI are also single-UI-threaded. This approach is nearly > universal across UI frameworks. That's not exactly true, or at least not exactly the same. For example, in JavaFX -unless I misread the source code-, one can safely invoke label.getText() from any thread, in SWT one can't. So if there were some annotations, the same piece of code might have to set different annotations for the SWT case (nothing, or some @RunInUIThread annotation) and the JavaFX case (@AnyThread). That is actually a pretty good example to highlight that ability to run in a non-UI Thread actually depends on the graphical library, so that annotations or interfaces or whatever to deal with that is framework specific. (In reply to Nikita Nemkin from comment #14) > Anyway, I'm -1 on having this in SWT. If every "useful" thing was added to > SWT, we wouldn't have JFace. The rest is not my area of expertise. I'm ok to move this to JFace, but I'm deeply convinced that this case and problem are heavily in SWT and that SWT should "embrace" it rather than reject it.
(In reply to Mickael Istria from comment #15) > > JavaFX and Android UI are also single-UI-threaded. This approach is nearly > > universal across UI frameworks. > > That's not exactly true, or at least not exactly the same. > For example, in JavaFX -unless I misread the source code-, one can safely > invoke label.getText() from any thread, in SWT one can't. > So if there were some annotations, the same piece of code might have to set > different annotations for the SWT case (nothing, or some @RunInUIThread > annotation) and the JavaFX case (@AnyThread). That is actually a pretty good > example to highlight that ability to run in a non-UI Thread actually depends > on the graphical library, so that annotations or interfaces or whatever to > deal with that is framework specific. My source is this: https://docs.oracle.com/javase/8/javafx/interoperability-tutorial/concurrency.htm > The JavaFX scene graph, which represents the graphical user interface > of a JavaFX application, is not thread-safe and can only be accessed and > modified from the UI thread also known as the JavaFX Application thread. I didn't find any synchronization in JavaFX properties implementation either (look at ObjectPropertyBase.set for example). The only difference from SWT is that JavaFX doesn't preemptively check for invalid thread access and invalid access might appear to work.
JavaFX is not thread safe when it comes to scene graph modifications, and that is checked strictly in runtime. So if you modify a observable expression which result in SceneGraph modification, the SceneGraph will throw out. At least that how it works in JavaFX8 :) By the way guys aren't we a little bit deviating from our main course ?
As mentioned in the gerrit, I think the concept of "can run outside UI Thread with annotation" seems flawed to me. The main problem is that this annotation does not solve the issue itself but adds another level of complexity, as in fact in any non trivial use-case the class can't tell its safe to not execute in a "non-ui-thread". Even though the annotation is present, it doesn't mean the code is safe to execute concurrently in another thread than the current one (let it be an UI Thread or not), even worse, the calling code might need a computation to update and thus has to wait for the result anyways and so on. From my point of view, this is more an API contract, if we have expensive computations (e.g. JDT content assist) the API should state if implementation have to be: a) thread safe / concurrent b) can expect to run in a specific thread c) ... whatever ... then the specific user can decide if he can an/or want to schedule work outside the current context and how to join back. As I find the code to handle this very repetitive, I have written some helper methods namely - static <T> T executeBusy(Callable<T> action) throws InterruptedException, ExecutionException (using the BusyIndicator) - static <T> T executeInUserInterfaceThread(Callable<T> action) throws InterruptedException, ExecutionException (either directly execute are syncExec based on thread state) don't know if it would be sufficient to provide those directly in the Display class, for me they simplified (reactive) execution a lot. I could even think about something like "executeInBackground" that works similar to the SwingWorker concept.
https://git.eclipse.org/r/c/platform/eclipse.platform.text/+/175291 / bug 561372 is yet another example here: we'd like to get some code declaring it's not requiring UI thread to let the manager/consumer run things in parallel. It's now hyperlink detection, after content-assist processors; but that can expand to all extension point that have so far let consumer assumed they'll run code in the UI Thread. In https://git.eclipse.org/r/c/platform/eclipse.platform.text/+/175291 , the suggested approach is more a marker interface, without payload. Typically just like an annotation except it shows up in class hierarchy (which is IMO not a big problem and actually more convenient as it's just a matter of checking for an instanceof). The issue with this approach, as we can see in the Gerrit patch, is that it requires to implement different adapters and so on. So maybe the interface should be a "AsyncableStateProvider" or something of that sort with a "requiresUIThread()" ? Or maybe there is already a relatively standard way in Java to declare that some code can run in any thread ?
(In reply to Mickael Istria from comment #19) > Or maybe there is already a relatively standard way in Java to declare that > some code can run in any thread ? In java all code can run in any thread so there is no need to declare it. Its a complete SWT concern that it requires a special thread. If an API is "swt-safe" it should better state this in an API contract or even better state that it is NOT guaranteed and code needs to take care of. One Example is the E4 @UIEventTopic that guaranties per contract that it will call the handler on the UI thread, all other ones can be called from any thread.
(In reply to Christoph Laeubrich from comment #20) > If an API is > "swt-safe" it should better state this in an API contract That's more or less the question of this bug. What is the best way to state this API contract? > or even better > state that it is NOT guaranteed and code needs to take care of. That's the theory. In practice, there are now lots of pieces of code and extensions everywhere that require UI Thread because the implementaton has allowed that from the beginning, so the UI Thread requirement is the default for many legacy extension point and we cannot just ask all extensions to suddenly declare the requirement that has been assumed for years. Those in-UI-Thread extension and extension points are the one to focus on. Note that more recent extension points (eg generic editor) and a few other older ones (hover) are actually assuming that all code should run in any thread. So this ticket doesn't apply to those.
But in that case an extension interface as proposed there (modernize API contract) would be the best choice in contrast to an annotation that no one is mandated or know about to respect as you have noted on other places. Another alternative (I used that for the PDE extension) is to make a clean cut, create a new interface and simply use an AdapterFactory to map old->new, in the case of SWT access its quite easy, you simply implement the new interface and wrap each call in a call to the swt thread. Most probably an (internal?) helper method similar to UISyncronize would help in keeping the implementation burden low.
