http://hg.netbeans.org/jet-main/file/tip/java.editor/src/org/netbeans/modules/editor/java/RunOffAWT.java

http://hg.netbeans.org/jet-main/diff/tip/csl.api/src/org/netbeans/modules/csl/editor/hyperlink/RunOffAWT.java

Another new copy created: http://hg.netbeans.org/main-silver?cmd=changeset;node=5b5c28b78bb7

*** Issue 173990 has been marked as a duplicate of this issue. ***

Bugs (about duplicated code) are depending on this issue and as such it shall have higher priority than just an 
enhancement.

+1

There used to be (and very likely still is) a similar helper method in J2EE utilities.

We should probably consider also non-cancel version of the call. 

Warning message is meant to be customizable?

We've got P2 issue #174198, which needs this as well. Thanks a lot!

Created attachment 89787 [details]
patch

Please review.

[JG01] Missing class Javadoc. (2x)


[JG02] Missing private null constructor.


[JG03] You cannot run "off Abstract Window Toolkit". You can run off the "[AWT] event queue [thread]", or EQ. Compare
java.awt.EventQueue for terminology.

[JG01-02] - I will add it

[JG03] - runOffEDT() or runOffEQ()?

JG03 - not sure (EventQueue Javadoc variously refers to "the dispatch thread of the system EventQueue" or "the event
dispatcher thread" or "the current AWT EventQueue's dispatch thread" or "the current AWT EventDispatchThread") but
whichever you pick it is probably better to spell it out, e.g.

runOffEventDispatchThread

I'd like to suggest that there is a bigger issue within this development.  What I see happening, is an attempt to
provide a low level API that runs some code in the background, watches it, and provides the user the opportunity to
terminate (not sure how you will make that happen reliably) or ignore (this seems perilous) the work.

While this is one way to manage the issue, it seems that over all, we are not focusing on the fact that the user needs
to be impacted the least possible by all of the work the IDE is doing.  These things that take a long time to do, should
be few and far between, and probably need very, very specific facilities to allow cancellation to work.

I don't really think that this API, in and of itself provides a great solution to this issue.  It seems like there
should be a class with abstract methods that users can implement and then plug their work parts into.

The SwingWorker kind of layout with "setup", "background-work", "post-background-work" is the kind of thing I am
thinking about.

I'll attach a class implementation which I think makes the next step to help manage the issue that we'd like background
processing to just happen in the background, and when it completes, something can change in the IDE, but there are not
so many times that the user should really be kept from doing other work, and editing in particular needs to be possible
in all but the most extreme cases, such as refactoring.

Created attachment 89796 [details]
An example class providing Future implementation for background processing.

When I reported this enhancement (was enhancement at that time), I did not include a particular example of an action
that should use the envisioned runOffAWT method. Let me, first of all, correct this mistake. This method is supposed to
be used by user invoked actions, that are expected to run fast (but, as the IDE is not a realtime system, it cannot be
guaranteed to run fast). An example is Go to Declaration. In vast majority of cases, this action should run very fast.
In some, hopefully rare (modulo scanning), cases it may take a while to execute. For me, Go to Declaration is a
foreground action - I wait for it to finish (when I use it to go to a method, I want to see that method). In case it
would take a long time, I may opt to cancel it and find the target of the action manually, but that certainly should not
be common. Do you really want to be able to e.g. edit the source, despite the fact that the new target can be opened at
any time? Note that in 6.0, this action ran completely in AWT thread, in 6.1 or 6.5 (do not remember which) a check for
scanning in progress was added, but if the scanning was not running, the action ran in AWT anyway. Note that the
runOffAWT method is not supposed to be used by action that are expected to take a longer time under "normal" circumstances.

Regarding the proposed ControlledProcessor class, I have a few comments/questions:
-what is the intended use of the class? What features should use it to achieve what result? What advantages it provide
compared to simple use of NB's RequestProcessor? Sorry, but I do not see much advantage of using this class (but I do
not know what particular usecases it should solve).
-I do not see the advantage of the setup(), performWork() and after() methods - these are executed in order in one
thread - what is the benefit of having separate methods over simply performing the actions in one method (or calling
separate methods manually, if that is appropriate for improving readability).
-SwingUtilities.invokeAndWait should be used with maximal care - it is one of the most dangerous methods I know of. Note
that in NB one can use Mutex.EVENT.readAccess(Action) (not that I would recommend using that).

BTW Mutex.EVENT.readAccess does not block when off EQ - it is like invokeLater. It is synchronous when on EQ.

Y01 Gregg, I know that the runOff method may easily be misused, but I rather see misuse of one API method than copy 
pasting the same code into many places in the IDE. Anyway that is not the question: The question is, what you mean by 
your comment? Do you believe that the patch, as produced by Tomáš Holý and after a bit of polishing, shall not be 
integrated at all? If so, you need to change API_REVIEW_FAST keyword into API_REVIEW before or when Tomáš Holý 
announces 24h countdown before integration.

Y02 I like Jan Lahoda's explanation of the method's purpose. Something like this shall be added to method's javadoc.

Y03 To avoid misuse, I suggest to add some detection of slowness. According to Jan's explanation following might work: 
Have a Map<Class<? extends Runnable>, Integer> that will remove(operation.getClass()) whenever the operation finishes 
faster than in 100ms. Otherwise if ((put(clazz, get(clazz) + time)) > 10s) /* at least two slow invocations in a row 
taking together longer than 10s */ LOG.log(Level.WARNING, ..., new Exception(clazz + " is too slow"));

Y04 Gregg, if you believe that your new class shall be used instead of here-in proposed patch, start new review or 
wait for conf call (which will have to happen if you change the keyword as suggested in Y01).

Y05 Don't use RP.getDefault(), use named RP with throughput one.

Y06 The Trivial implementation is buggy, does not wait for end of background processing.

Y07 The same test shall test both implementations. You can subclass the test in progress.ui and just provide different 
provider.

My knowledge of all internal methods of netbeans and how requests for work are dispatched for various reasons, is not
that great, so you will have to help me understand which API I am missing out on the use cases for.

The example class that I attached, it about providing an interface for GUI actions to use to access the underlying
workings of the platform (and IDE in this case) so that if there needs to be some rewiring or rearchitecting of how
things interact with the core of netbeans (the big problem with the EDT encountering the scanning/indexing locks is an
example of wide spread problem because there was no "standard" way to do that "correctly").

I am suggesting Future because it means using a core Java standard for doing something in the background and getting the
results from multiple thread contexts.

I really want to push the idea of EDT listeners looking like

protected void performAction( Node[] activatedNodes ) {
    ControlledProcessor cp = new ControlledProcessor() {
       ....
    };
    RequestProcessor rp = ...
    rp.post( cp );
}

The idea being that with the ControlledProcessor (or something like it) in place, there is
a way to completely manage the orchestration of tasks that need to use background execution
and EDT work as well.

I did not make setup() and after() run in the EDT, but we could decide that was the right thing to do so that it was
more inline with SwingWorker (That's really the model I am trying to push).

Being a Future means that you can pass the object of to some other thread of execution that can use Future.get(...) as
needed to get the result.  In the case of these progressively exposed control mechanisms (first busy cursor, then
dialog), overriding get() logic can use timed get() calls to implement the progression of control.

There are a lot of things in netbeans that do already exist to manage this issue, but all of those things, to me
anyways, don't fit together into a model that helps manage this issue.  Instead, every developer has to make judgements
based on their knowledge of the issues to create the "correct" interaction with the system.

I've done tons of GUIs using Jini services as the "engine" that the GUIs talk through.  That means that every action and
interaction is a remote call with all kinds of possible partial failures to deal with.  I've always been able to just
use the SwingWorker model (extended with the features that I've discussed on the list regarding context class loader and
JAAS Subject etc) and not had problems with that affecting how things work.

I use control disables on action invocation to keep things from being requested twice (this makes locking a lot less
needed), and the stuff just works.

Hopefully, I've answered all the questions that were asked, I tried to cover all the things that were behind this
suggestion.

I'm not asking for the IDE to be rewritten, I'm asking for you guys to carefully consider how much control (or lack
thereof) you make developers have to implement themselves to get stuff done and to make sure the IDE is usable and
performant with all the things that are happening at the core as well as in the background related to individual features.

RequestProcessor is a fine thing to use to manage when things get done and how much parallel activities are going on. 
Adding a class to wrap the work so that the EDT and non-EDT activities are programatically together, and logically
sequenced, I thinkm, greatly simplifies the success rate of the developer. 

Created attachment 89923 [details]
updated patch

First of all thank you for this API, I like it and I vote for having it. Now my questions:

VS01: Can I use this API for tasks that are not cancelable?
VS02: And for tasks that are cancelable up to a certain point, which when reached makes the task non-cancelable?
VS03: How is the UI going to look for these tasks? Should tasks somehow signal their state to the UI?
VS04: What if a task is canceled, but does not finish? How is the UI going to react to this situation?

Btw. I think that the vast majority of tasks that will use this API are going to be non-cancelable tasks. Tasks that run
fast under normal circumstances and hence are not implemented in a way that allows canceling.

[MM01] Shouldn't the class be split into a generic org.openide.util part and a java support specific part? I know we can do it anytime later but IMHO we could 
agree to do it even now.

Does anyone have any further thoughts or comments on what I added to this issue?  I still sense that it seems okay to
netbeans developers to run "anything that is fast" on the EDT, when the reality of the impact that has when 10 modules
are running 20ms tasks is a 200ms delay in EDT availability.  The IDE is expandable and each module can have a feeling
that their work will run fast, but the reality is that if 10 modules react to an editor event and quickly stick work in
the EQ that is just 20ms that suddenly there is the 200ms delay between events that really starts to become visible and
make the IDE feel clunky.

I really feel like the overall platform has to be considered here, and individual modules and bits of code acting on
events must consider that anything they run on the EDT has the ability to greatly impact the user experience.

"Does anyone have any further thoughts or comments on what I added to this issue?" - I don't.

"Does anyone have any further thoughts or comments on what I added to this issue?" - Gregg, I really like one of your 
statements: "AWT thread shall be used only for painting". It is obviously overstated goal, but such are all the tasks 
I like. However originally this issue is about removing duplicated code, so commenting on your statement or your diff 
seems to be out of the scope of this issue, at least from my point of view.

jtulach: I understand what this issue is about.  What I'm worried about is that this solution is too simple and doesn't
provide enough structure and is not focused enough to help deal with the bigger issue of keeping people from doing
inappropriate work on the EDT.  This issue helps address how timeouts and user cancellation are presented, but doesn't,
I feel, address the needs of the developer to have this accessible to them in a way that they can truely make
cancellable tasks as well as "see" how to structure the work as a sequence of "setup GUI for task", "perform task that
might run for a while", "reconfigure the GUI after the task".  I understand that this issue is a bit lower level in
detail, but I am concerned that people will still just choose to run "whatever" stuff on the EDT, and only use this API
when they want "cancellation" or a "man this is running a long time" capabilities.

So, I'm trying to stress that if you provide the API this issue is about, that you really could raise the bar and
provide something like what I suggested and say that this style of processing is preferred.  I really think this would
help people be much more clear on how to deal with events from the GUI and help make the processing on the EDT be much
reduced to help the usability of the IDE (and platform) was well as to provide future ability to make changes in how
threading is handled in more central places.

Well, we could move the discussion that Gregg wants to have back to the
   [openide-dev] EDT Was: New Language Support Issues
thread. 

There is a social issue I've been thinking about bringing up.
People who are co-located can discuss things "unproductively"
at lunch or by the coffee stand or at the bar. For those
of us who are isolated it becomes a bit frustrating since 
most of the communication venues are too formal or structured.
So we end up with all this stuff we want to talk about but
no outlet for it.

I suggest that the discussion about Gregg's proposal moves to a new issue (instead of openide-dev) that would be created
by Gregg and "maintained" by Gregg. Gregg if you want your proposal to materialize you must provide close to final patch
(not just a sketch) + answer the concerns raised by the people in the issue.

Re VS01-03: Interesting questions. For tasks that would not be cancelable at all, a null cancel parameter could be used.
But, I think that most of the tasks would be "cancelable up to a point" - I will try to think about that (unless Tomas
objects). Anyway, from the user's point of view, I think that we should try to make the tasks as cancelable as possible.
Imagine that the user invokes a quick action and is blocked for a long time without possibility to cancel the action. I
would not like such a situation (even if it was rare).

VS04 (IMO): the canceled task will not probably finish immediately (it may wait on a condition and may not be able to
check for cancel value), but should not have any effect. The UI should disappear.

I think only cancelable task should be supported. If this method should support also non-cancelable task calling this
method is not much different than blocking EDT. UI will be painted but user will not be able to do anything than
terminate NB.

I have created an issue and put a module project up as an attachment for people to look at for now.

http://www.netbeans.org/issues/show_bug.cgi?id=175342

Created attachment 90269 [details]
updated patch

After discussion with jtulach I made small change. If the task is canceled the method waits until task is finished. If
it does not finish in 1s an exception is thrown. This should help in situation when it is necessary to prevent return
from runOffEDT before task is finished. If there are no objections I will integrate after 24h.

I am not sure about this last change. Considering my usecase with Go to Declaration, it is very probable in this context
that the task will not finish in the limit. The reason is that if the action is block for so long (~10s), it is probably
because an indexing is in progress. In such a case, the action is waiting on a lock and is not checking cancel. It
checks the flag when the scan finishes and the task is started, but this quite likely will not be in the time limit. So
this new updated implementation does not seem useful for the Go to Declaration usecase.

OK, so waiting for canceled task should be probably optional.

Created attachment 90278 [details]
updated patch - waiting for canceled task is optional

Looks like the amount of method arguments is getting more and more complicated. Here is a way to eliminate this 
complexity:

Y11 Use cummulative factory or builder (http://wiki.apidesign.org/wiki/CumulativeFactory). In this case it would be:

public class RunOffEventQueue {
  public static RunOffEventQueue create(Runnable r, String decr, AtomicBoolean cancelOp);
  public RunOffEventQueue waitForCancel(boolean wait);
  public RunOffEventQueue waitCursorAfter(int ms);
  public RunOffEventQueue ....(...);
  public void execute();
}

with a sample usage: RunOffEQ.create(...).waitCursorAfter(1000).execute(). This is just an advice, if you do not like 
it integrate the patch with static methods.

I am fine with the optional waiting for cancel.

Thanks for review.

core-main #47c0ebae455e

Integrated into 'main-golden', will be available in build *200911030222* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/47c0ebae455e
User: Tomas Holy <t_h@netbeans.org>
Log: #170882: runOffAWT method