This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 172694 - Add API for synchronous/asynchronous calls of view model methods.
Summary: Add API for synchronous/asynchronous calls of view model methods.
Status: RESOLVED FIXED
Alias: None
Product: debugger
Classification: Unclassified
Component: Code (show other bugs)
Version: 6.x
Hardware: All All
: P1 blocker (vote)
Assignee: Martin Entlicher
URL:
Keywords: API, API_REVIEW_FAST
Depends on:
Blocks: 172060
  Show dependency tree
 
Reported: 2009-09-21 15:34 UTC by Martin Entlicher
Modified: 2009-10-02 16:22 UTC (History)
3 users (show)

See Also:
Issue Type: TASK
Exception Reporter:


Attachments
API part of this change. (23.93 KB, text/plain)
2009-09-21 15:41 UTC, Martin Entlicher
Details
Complete diff of the current API change, including implementation and a unit test. (61.51 KB, text/plain)
2009-09-22 17:28 UTC, Martin Entlicher
Details
A modified API for threading management (18.67 KB, text/plain)
2009-09-23 22:12 UTC, Martin Entlicher
Details
The final proposed API change - as already described (18.78 KB, text/plain)
2009-09-24 23:20 UTC, Martin Entlicher
Details
The final API change including implementation and unit test. (62.50 KB, text/plain)
2009-09-24 23:21 UTC, Martin Entlicher
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Entlicher 2009-09-21 15:34:35 UTC
See http://www.netbeans.org/issues/show_bug.cgi?id=172060

It's necessary for the clients to be able to define threading of their view models. Some clients (C/C++ debugger) need
synchronous model (methods called in AWT), some (JPDA debugger) need asynchronous model (methods called in an RP so that
AWT s not blocked by expensive JDI calls).
Comment 1 Martin Entlicher 2009-09-21 15:41:13 UTC
Created attachment 88027 [details]
API part of this change.
Comment 2 Martin Entlicher 2009-09-21 15:43:06 UTC
Please review this compatible API addition that allows clients of view models to define threading.
Comment 3 Martin Entlicher 2009-09-21 15:46:47 UTC
I'm finishing with the implementation part, the tests will follow shortly afterwards...
Comment 4 ivan 2009-09-21 21:41:14 UTC
What NB is this slated for? 6.8 or 6.next?

Hmmm ...

What I'd like to see a comprehensive analysis of MT issues and debuggers, both external and
internal. W/o such an anlaysis in one mind it's hard to judge the wellness
of an individual API proposal. (just for taste ... shouldn't ThreadProvider.DEFAULT_RP
also be used to submit debugger actions? See further below)

I've been meaning to do such an analysis and Vladimir (vv) has been on my case too
but I'm knee deep in alligators trying to get sunstudio working again after switching to
nb 6.8 from 6.5.1 and other issues.

In broad terms I see any "debuggergui" code as a body of code which is entered and exited
from various places. All of these execution paths touch shared data ("intermediate models")
so some kind of synchronisation mechanism needs to be established. The one I adopted was serialization
of all execution paths through a single thread. In lieu of something like
ThreadProvider.DEFAULT_RP the awt thread was the only choice. So  from this POV
DEFAULT_RP is a step forwrd.
However, there is also the networking infrastructure that we use in sunstudio and it has it's _own_
worker threads and what I haven't the time yet to see how that will interact with
ThreadProvider.DEFAULT_RP.

I also see a different set of problems as the focus of these solutions.
Lot of modelview code and other mechanisms (the lingering evaluationthreads
come to mind) seem to be solving the problem of "if I ask for data (i.e. do a pull)
and the data takes a long time, what do I do"?

In sunstudio we don't have that problem and what I'm focused on
is what I mentioned above, "how to keep the various execution pathways from 
touching the same "model" data ("model" here is much more than
is presented via modelview) in a racey way". We don't have a
"takes too long" problem (at least in refreshing the view) because
the engine (dbx, gdb) "pushes" data into this "model" and invalidates it.
Then modelview pulls it. These pulls are always quick so synchronous is OK.

So I'm very curious as to how jpdadebugger solves the synchronisation
problem. What is it's threading "big picture"?

Another area has to do with the execution flow of actions 
and the reverse flow of information from engines to views. 
Ideally these flows should be completely decoupled like a in perfect
MVC architecture. This would then dictate for example a thred to
handle the flow of executionin each direction.
Bbut in practice we don't have perfect MVC and actions touch the shared
data as well so that makes the two thread approach iffy. This is why
I suggested that actions also perhaps be dispatched on DEFAULT_RP.







Comment 5 ivan 2009-09-21 23:08:54 UTC
What NB is this slated for? 6.8 or 6.next?

Hmmm ...

What I'd like to see a comprehensive analysis of MT issues and debuggers, both external and
internal. W/o such an anlaysis in one mind it's hard to judge the wellness
of an individual API proposal. (just for taste ... shouldn't ThreadProvider.DEFAULT_RP
also be used to submit debugger actions? See further below)

I've been meaning to do such an analysis and Vladimir (vv) has been on my case too
but I'm knee deep in alligators trying to get sunstudio working again after switching to
nb 6.8 from 6.5.1 and other issues.

In broad terms I see any "debuggergui" code as a body of code which is entered and exited
from various places. All of these execution paths touch shared data ("intermediate models")
so some kind of synchronisation mechanism needs to be established. The one I adopted was serialization
of all execution paths through a single thread. In lieu of something like
ThreadProvider.DEFAULT_RP the awt thread was the only choice. So  from this POV
DEFAULT_RP is a step forwrd.
However, there is also the networking infrastructure that we use in sunstudio and it has it's _own_
worker threads and what I haven't the time yet to see how that will interact with
ThreadProvider.DEFAULT_RP.

I also see a different set of problems as the focus of these solutions.
Lot of modelview code and other mechanisms (the lingering evaluationthreads
come to mind) seem to be solving the problem of "if I ask for data (i.e. do a pull)
and the data takes a long time, what do I do"?

In sunstudio we don't have that problem and what I'm focused on
is what I mentioned above, "how to keep the various execution pathways from 
touching the same "model" data ("model" here is much more than
is presented via modelview) in a racey way". We don't have a
"takes too long" problem (at least in refreshing the view) because
the engine (dbx, gdb) "pushes" data into this "model" and invalidates it.
Then modelview pulls it. These pulls are always quick so synchronous is OK.

So I'm very curious as to how jpdadebugger solves the synchronisation
problem. What is it's threading "big picture"?

Another area has to do with the execution flow of actions 
and the reverse flow of information from engines to views. 
Ideally these flows should be completely decoupled like a in perfect
MVC architecture. This would then dictate for example a thred to
handle the flow of executionin each direction.
Bbut in practice we don't have perfect MVC and actions touch the shared
data as well so that makes the two thread approach iffy. This is why
I suggested that actions also perhaps be dispatched on DEFAULT_RP.







Comment 6 Martin Entlicher 2009-09-22 17:20:38 UTC
This is slated for 6.8 and I'd like to integrate this at the end of this week so that it can get into 6.8 beta.

The brief MT analysis as I see it is:
JPDA debugger need to run almost everything lazily, because nearly every call to com.sun.jdi APIs talks to the debuggee
and waits for the results, thus making the calls blocking and very dangerous to call in AWT.
It seems to me that C debuggers works in an opposite approach, their calls are not blocking and therefore suitable for
execution in AWT.

While it's certainly true that every model implementation can put the execution off the AWT when it needs to, in
practice this is extremely inconvenient (in JPDA case), because we have so many entry points to com.sun.jdi API that
it's best to resolve the transition from AWT to lazy evaluation centrally at once place.

It's actually a good point, that ThreadProvider.DEFAULT_RP should be used for debugger actions (you mean
ActionsProvider.postAction() I guess, where RequestProcessor.getDefault() with throughoutput of 50 threads is used). But
due to a big fragmentation of debugger modules (ThreadProvider.DEFAULT_RP is in a different module than
ActionsProvider.postAction()) this is hard to achieve without additional module dependency. Also, a single-threaded RP
for actions might not suit to everyone. That can be solved e.g. by adding ActionsProvider.setPreferredRP(RequestProcessor).

> I see any "debuggergui" code as a body of code which is entered and exited from various places
(threads)
Yes, I agree that currently it looks sort of chaotic. Some methods on view models are called in AWT, some in RP threads,
actions in other RP threads, and occasionally there might come refresh in even other thread. JPDA debugger does not have
a problem with that, it uses concurrent locks for the access to shared data. It uses RequestProcessor with throughoutput
of 5 internally. NetBeans as a whole look like a highly MT environment.

This API change is trying to bring some control to that and from what you say it seems to be a step in the right
direction, but not sufficiently enough yet.

> networking infrastructure that we use in sunstudio has it's _own_ worker threads
There is a space for extension. This is why AsynchronousModel.asynchronous() return ThreadProvider and not just
true/false. I had in mind that you might have your own preferred threads and would like to be called in them.
Would it be useful for you if we add ThreadProvider.createExecutor(ExecutorService es) (returning an instance of
ThreadProvider) and then call you in that ExecutorService? Or would you need something totally general that would take
Runnable and you would execute it where you want?

The other problem you mention: "if I ask for data (i.e. do a pull) and the data takes a long time, what do I do"
this is solved in the current viewmodel when it calls models in RP. When UI asks for children, but we compute them
asynchronously, a wait icon is displayed instead and after we get the data, children are displayed. When UI asks for
values, but TableModel.getValueAt() is processed asynchronously, we display "Evaluating..." string until we get the
data. This is one more reason for the necessity to preform the asynchronous work at one place, where UI is constructed.

Basic MVC architecture in NetBeans is usage of RequestProcessor. The idea is that you get UI input from AWT, post a work
into RP and then if you need to update UI as a response, you may switch to AWT back again. ActionsProvider and
AsynchronousModel should help you with that and you're probably right that they should provide the same level of
flexibility. With ActionsProvider.setPreferredRP() you could return ThreadProvider.DEFAULT_RP there and be sure that
you'll be called in one RP with throughoutput of 1.

Comment 7 Martin Entlicher 2009-09-22 17:28:06 UTC
Created attachment 88138 [details]
Complete diff of the current API change, including implementation and a unit test.
Comment 8 ivan 2009-09-22 21:49:38 UTC
Thanks for the explanation Martin.
I wish I had is many years ago :-)

- Is DEFAULT_RP strictly necessary for solving the issues at hand?
  In some sense, it's all over the code so the answer is yes.
  However, is <action>.setPreferredRP(), the best solution?
  It helps with avoiding module dependencies only if engines call it but you had mentioned that 
  for JPDA centralizing stuff helps a lot. It follows that calling setPreferredRP() in 
  a central location would introduce the dependency from debuggercore to modelview 
  that you wanted to avoid. (time for Lookup?)

- Thanks for outing the "chaos" :-). 
  As chaotic at it might be an attempt to document it would be worthwhile.
  That any code has (fine grain?) locking gives me the hee-bee-gee-bees.
  Do do a decent job of locking one has to have good grasp of entry and exit points but
  I"m still looking (haven't looked hard enough) for a static analysis tool that can 
  answer questions like: "find all implementations of abstract methods of classes which
  aren't mine" This should find external callbacks for example.
  Until I can get a better grasp on things I'll stick to serialization through a single
  thread.

- As for mixing worker threads ... I _think_ nothing as fancy as
  ThreadProvider.createExecutor(ExecutorService es) is needed.

  Sunstudio uses a class called Notifier. It's a "view" unto an event dispatching mechanism
  so in principle I can create an implementation of Notifier connected to the default RP.
  that would mean that if I get a network message from an external debugger instead of
  calling invokeLater() I'll call RP.post() of some sort.

  But what guarantees are there that modelview always uses the DEFAULT_RP? 
  Or put another way when does modelview use ThreadProvider.DEFAULT_RP and when
  does it use ThreadProvider..getRequestProcessor()? What happens if they are not the same?

 - The naming in class ThreadProvider is so inconsistent that it's a bit hard to follow:
  - Why is the type of CURRENT_THREAD ThreadProvider? (OR why is it not calls CURRENT_THREAD_PROVIDER?)
  - Why is the type of DEFAULT_RP ThreadProvider (OR why is it not called DEFAULT_THREAD_PROVIDER?)
  - the static constructing call is then called createRPProvider that sets 'rp'
  - then there is a method isCurrentThread that tests 'rp'.
  So, to make it short three word: ThreadProvider, CurrentThread, RequestProcessor are being used
  interchangeably.

- I'm remembering some hairy constraints which Hans (or was it Martin?) worried about.
  Because of model filtering the flow of execution to get or set something will snake through
  several models which might be owned by different debugger implementations. What if each
  of these implementations wants to work with a different threading model?

- On MVC and MT. There are three phases in MVC: action: C->M, notification: M->V and pull: V<-M 
  Additionally the M might change "spontaneously", I.e. not as a result of an action but 
  arrival of message from engine or internal programmed state. So, given the aboev
  phases of action, notification and pull how do you switch back and forth between AWT and RP?

  The above is classic MVC. But we use a variation because of inter-process latency with the following
  phases: "push" MVC: action: C->M, push: M->V

  Another dimension is that the M is typically a proxy for a M' which it shadows. You end up
  with a cascade of models. For example in how sunstudio uses debuggercore the cascade of models is:
  - swing widgets get data from ...
  - Node, which get data from ...
  - modeview models which get data from
  - dbxgui's internal model which shadows
  - what the dbx engine sends it.
  So the actions, notifications and pull (or pushes) have to traverse all these layers. 
  I have think a bit more about how RP's fall into the above picture :-)

  

  


  
Comment 9 Martin Entlicher 2009-09-23 18:02:04 UTC
- DEFAULT_RP is not necessary to solve this issue. It was just a recommendation by Vladimir in issue #172060 for
usability reasons. It has similar effect as returning ThreadProvider created by:
ThreadProvider.createRPProvider(new RequestProcessor("MyRP", 1))
For solving this issue is essential ThreadProvider.CURRENT_THREAD, which means that model methods are called in AWT
(synchronously with the UI).
Yes, I want to avoid dependency of "api.debugger" module on "spi.viewnodel", because otherwise it would not make sense
for "spi.debugger.ui" to be a separate module.

- The more I think about current "chaotic" behavior, the more I tend to agree with Vladimir, who expected
Swing-compatible behavior - to call everything synchronously in AWT by default. This would really be much simplified
default behavior, which also was there cca 5 years ago, when this was changed to the current state (because of JPDA).
The correct approach would have been to introduce such AsynchronousModel 5 years ago instead of changing the default.

- > But what guarantees are there that modelview always uses the DEFAULT_RP? 
  Or put another way when does modelview use ThreadProvider.DEFAULT_RP and when
  does it use ThreadProvider..getRequestProcessor()? What happens if they are not the same?

modelview always use the ThreadProvider that you return from AsynchronousModel.asynchronous(...) It's guaranteed that if
you return ThreadProvider.CURRENT_THREAD, your models implementation will be called synchronously in the current thread
(AWT probably, unless someone decide to call e.g. Node.getChildren() in a non-AWT thread. But the UI calls it in AWT).
If you return ThreadProvider.DEFAULT_RP (or ThreadProvider.createRPProvider(..)), that default RequestProcessor will be
used to call you and UI will display waiting icon or "Evaluating..." string until your methods return.
ThreadProvider is used just as a extensible wrapper for RequestProcessor (or meaning AWT without RequestProcessor).

- To the naming - I was thinking about some simplification and also had a conversation with Yarda a while ago. The
brain-storming we had was very productive and I will update the API soon. I will simplify it even more while adding more
flexibility...

- What if each of these implementations wants to work with a different threading model?
This is what AsynchronousModelFilter is for. It can change the threading model.
Also to this - since it's problematic to automatically merge by viewmodel several implementations of AsynchronousModel
(if someone happens to register more than one impl.), we might remove it from API and have it only for the default
begavior. Clients would implement only AsynchronousModelFilter, where they can override the default (or results of other
filters).

- To MVC: I usually solve this by synchronization of the shared data by "synchronize" keyword and use
SwingUtilities.invokeLater() to switch to AWT (possibly passing final computed variables instead of synchronization) and
then RequestProcessor.post() to switch to RP, again possibly with final variables to pass from AWT. Also synchronized
getter/setter methods on models can work fine allowing setters be called during computation in RP and simple getters in AWT.

- To the cascade of models you describe - modelview interacts with nodes and swing widgets, so you probably do not have
to care about this. Then you'll probably get what dbx engine sends in some dedicated thread (you write about Notifier
thread) from that I'd process the events in RequestProcessor - in the same that's used by the viewmodel. - Just an idea...
Comment 10 Martin Entlicher 2009-09-23 22:12:01 UTC
Created attachment 88238 [details]
A modified API for threading management
Comment 11 Martin Entlicher 2009-09-23 22:24:49 UTC
I've prepared a modified (and sort of reduced) version of the API change.
I've realized that since we have a default (which I've decided to change back to AWT where it was ~5 years ago), the
AsynchronousModel is not necessary and AsynchronousModelFilter is enough. AsynchronousModelFilter.asynchronous(...) gets
the original Executor (or the one returned by another AsynchronousModelFilter).

ThreadProvider is replaced with Executor, since this gives what I originally had in mind - provide a way to run your
models wherever you want. CURRENT_THREAD and DEFAULT are constants meaning the same as former
ThreadProvider.CURRENT_THREAD and ThreadProvider.DEFAULT_RP. So the naming should be more clear now.

Actions are called in AWT if you override ActionsProvide.postAction(). And postAction() allows you to execute
"doAction()" wherever you want (e.g. in AsynchronousModelFilter.DEFAULT). Therefore is seems to fulfill all requirements.

Do you have any comments on this? Do you like it better?
Comment 12 ivan 2009-09-24 06:16:28 UTC
OK, let's see if I understand correctly.

- Everything is switched back to async model (i.e. call on eventw) & we know
  how to get actions to be sync or async.

- We won't have to change code in SunStudio.

- JPDA will now be forced to use the new AsynchronousModelFilter to divert things to RP's
  as it needs to. That seems like a lot of work and it has to be applied to 
  other debuggers too? Brave man.

- AsynchronousModelFilter.asynchronous() is called for each (type of field X node)
  and it returns an executor used for making that call.
  - Is the information that it returns cached anywhere?
  - It seems that returning CURRENT_THREAD is equivalent to returning 'original'.

- You said that if DEFAULT_RP is returned from asynchronous() that waiting icons will
  be used. Part of my "big picture" is that we eventually want to move dbxgui off
  of the eventQ as well (as VV suggested) and use an RP but we don't need all the 
  lazy mechanisms and automatic waitings that would come with DEFAULT_RP.
  Put another way there are two different concerns:
  - which thread is the "debugger thread" (awt or some RP shared with modelview and actions)?
  - is the debugger quick or will it potentially block for a long time?
  It seems to me these two concerns have been combined here.
  
Comment 13 Jaroslav Tulach 2009-09-24 11:57:18 UTC
This version is definitely more slim than the previous one. There are just two things I'd like to see to be changed:

Y02 (I am using 2 as this is less important): Usage of enums in APIs has a problem - you cannot safely extend the list 
of constants. That is not source compatible, code which uses switch(enumValue) { case:...; case:...; // no default } 
will not compile. I'd rather replace the enum with something else. java.lang.reflect.Method comes to my mind first. 
Ugly, but better than enum. Or rather than that I would recommend use of compile time @annotations on class or 
individual methods, but Martin already refused this as too complicated. Alternative proposal is to use enums in a way 
that does not lead to switch use. Something like:

 static enum CALL { DEFAULT, DISPLAY_NAME, SHORT_DESCRIPTION, VALUE, CHILDREN }
 Map<CALL, Executor> asynchronous(Executor original, Object node) throws UnknownTypeException;

In this style adding new constants is fine. If the code does not know it, the retValue.get(NEW_CONSTANT) will be null 
which will mean for the system to use retValue.get(DEFAULT). Moreover this later style can lead to more declarative 
programming (is there any reason to have different executors for different nodes anyway)?. The creation of the 
Map<CALL,Executor> could happen just once, and then the Map<CALL,Executor> reused again and again.



Y01 I am really not comfortable to see that behaviour we have for last five years is about to change. That is too 
dangerous. Please find a way for people to have the behaviour of 6.7 version if they don't change their registrations 
and get the new 6.8 if they do something. The "something" can for example mean - if there is an implementation of 
AsynchronousModelFilter (that clearly indicates they are coding against 6.8 version). By default, keep the behaviour 
of 6.7. Otherwise you are just needlessly shaking the amoeba (http://wiki.apidesign.org/wiki/Amoeba).
Comment 14 Jan Lahoda 2009-09-24 12:22:38 UTC
Re Y02: could you please be more specific about the switch code that would not compile if an enum constant would be
added? If no case matches the result of the given expression, and there is no default label, the switch statement does
nothing, to my knowledge (JSL 14.11). I do not know about any rule that would require switch to contain cases for all
(enum) values or a default label. Check also the "Discussion" section in JLS 14.11.
Comment 15 Martin Entlicher 2009-09-24 14:54:13 UTC
Y02: Thanks for your comments Honza, I've verified the actual behavior. And for instance following code does *not* compile:
public class EnumTest {

    static enum CALL { DISPLAY_NAME, SHORT_DESCRIPTION, VALUE, CHILDREN }

    public static void main(String[] args) {
        CALL c = CALL.DISPLAY_NAME;
        String s;
        switch (c) {
            case DISPLAY_NAME:
                s = "name"; break;
            case SHORT_DESCRIPTION:
                s = "description"; break;
            case VALUE:
                s = "value"; break;
            case CHILDREN:
                s = "children"; break;
        }
        System.out.println("s = "+s);
    }
}
It says that variable s might not be initialized. Therefore I do not see problems with future extension of this enum and
I'd like to keep it there. In my view the proposed Map<CALL, Executor> is unnecessarily complicated for clients to
implement.

> is there any reason to have different executors for different nodes anyway
There is no reason not to allow it. And yes, there is a use-case. Debuggers (e.g. JPDA debugger) can cache information
is retrieves from the target machine. If it retrieves the information for the first time, it's important to do it in RP,
since it can take a long time. Afterwards they can return cached info in AWT. Thus they can even return different
executors for the same nodes in different situations.

Y01
Well, after all, I accept that it's dangerous to change it now. I've looked into other debugger implementations we have
in NetBeans and realized that PHP and Python should not have problem with synchronous (AWT) model, but Ruby and probably
also Bpel would prefer to be called in RequestProcessor I guess. We could easily define AsynchronousModelFilter for
them, but we can not adjust other debugger implementations that might exist. While it's true that from the architectural
point of view it would be best to return the default state to the original AWT, but we must also keep compatibility. And
it seems to be more important to keep current state (since we have more clients now) than to be compatible with the
state we had 5 years ago.

So we'll keep the default threading as it is now.
Comment 16 Martin Entlicher 2009-09-24 15:26:31 UTC
To Ivan:

- Well, I'm sorry for changing the defaults. Since we're afraid of stability it was decided to keep compatibility at the
price of architectural clean solution. Thus I will change the defaults to AsynchronousModelFilter.DEFAULT for CHILDREN
and VALUE enums and keep AsynchronousModelFilter.CURRENT_THREAD for DISPLAY_NAME and SHORT_DESCRIPTION.

- Thus you do not have to change anything if your code works fine in NB 6.7. If you'd like to use AWT, implement
AsynchronousModelFilter and return AsynchronousModelFilter.CURRENT_THREAD from asynchronous() method.

- The executor returned from AsynchronousModelFilter.asynchronous() is not cached, the asynchronous() method is called
for each node before we call the appropriate method. Returning CURRENT_THREAD is equivalent to returning 'original' only
if there does not exist any other implementation of AsynchronousModelFilter sitting in between.

- As soon as you move execution to RP, you will want the lazy mechanism I guess. Otherwise you risk blocking of AWT
thread for an indefinite amount of time. RP does not have to have a free thread to process your task at the moment and
if we wait for it in AWT, we'd block the UI. The waiting icon is not displayed if your task in RP finish soon enough.
Currently we wait 200ms in AWT and if the task finishes, we display the children synchronously. For values the time is
shorter (25ms), since there are typically many more values in rows than expanded nodes with children.

- The API should help you to actually define the "debugger thread". It depends on the debugger implementation to choose
one and the rule I use is - when I do not have a control of how long it will take (e.g. reading from a socket or from
filesystem), use RP. If there's a small amount of code that only provides computed information, use AWT.
Comment 17 ivan 2009-09-24 18:44:48 UTC
Keeping the current state is OK. I was surprised that
you wanted to change it too. 

We'll have to start using AsynchronousModelFilter it looks like.
VV are you up to doing that?
Comment 18 Jaroslav Tulach 2009-09-24 20:39:14 UTC
Ignore Y02, you are right. The enums are not as bad as I have thought. Sorry.
Comment 19 Vladimir Voskresensky 2009-09-24 21:38:19 UTC
to Martin: VV1: It is fine if you leave current state as is (because release of 6.8 is so close). But I'd like to
propose to add these filters in all nb debuggers in trunk anyway (they will return correspondent current values) => this
allow us to change behavior in 6.9 to more correct one "core" level and do not break existing nb debuggers which already
defined their filters.

to Ivan: VV2: I'm ok to add filter into dbx.
Comment 20 Vladimir Voskresensky 2009-09-24 21:43:12 UTC
VV3: there are some forgotten  ThreadingInfo (instead of Executor) in javadoc of AsynchronousModelFilter.asynchronous
Comment 21 Martin Entlicher 2009-09-24 23:20:53 UTC
Created attachment 88318 [details]
The final proposed API change - as already described
Comment 22 Martin Entlicher 2009-09-24 23:21:42 UTC
Created attachment 88319 [details]
The final API change including implementation and unit test.
Comment 23 Martin Entlicher 2009-09-24 23:34:39 UTC
Thanks for comments, I've attached the final API change + implementation + unit test.
The implementation was tested on JPDA debugger and the unit test verifies the default threading.
The Javadoc specifies the defaults.

VV1: The main concern regarding compatibility were not debuggers we have in NetBeans distribution (these are easy to
adapt by adding simple implementations of AsynchronousModelFilter), but other debuggers that may exist elsewhere and
that we do know know about. Therefore from this point of view nothing changes after 6.8.
We must specify the default behavior in the API now and if we change it afterwards, it would be even worse incompatible
change.
So, since it was considered not to change the behavior now, I will not change the behavior in the future. From now, it's
set in the APIs.

VV3: Corrected, thanks.

If you're fine with the final shape of the API, I'll integrate it tomorrow.
Thanks.
Comment 24 Martin Entlicher 2009-09-25 15:05:04 UTC
Thanks for review.
This API change was integrated in changeset:   146886:798ed120725a
http://hg.netbeans.org/main/rev/798ed120725a
Comment 25 Quality Engineering 2009-09-27 22:34:20 UTC
Integrated into 'main-golden', will be available in build *200909270201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/798ed120725a
User: mentlicher@netbeans.org
Log: #172694 - AsynchronousModelFilter API added. Simple implementation in JPDA is used, providing it's preferred RequestProcessor.
Comment 26 ivan 2009-10-01 21:57:38 UTC
I'm a bit confused.
issue #172060 (setters no longer on eventq) initiated this API change.
Martin came up with a brave plan, but Yarda showed concern about bwd compatibility
and the plan was simplified.

Simultaneously issue #172702 was filed to deal with an analogous problem for
enabling and disabling. Here Martin undid a change made several years ago 
and made the callback go back to eventq from the RP.
Question: How come "Yardas concern" for bwd compatibility doesn't apply for 172702? 
Comment 27 Martin Entlicher 2009-10-01 22:24:50 UTC
Yes, we're inconsistent :-(
Unfortunately, it was my mistake that I did not leave default API behavior synchronous (4 - 5 years ago) and did not
propose something like asynchronous model at that time. The reason was mainly that we wanted to act quickly, debugger
was in a bad state, any delay in communication caused a freeze of AWT. Our first priority was to fix Java debugger. And
since it caused fatal defects we thought like: who would ever want to call debugger in AWT?

Now we did not have enough courage to change the behavior back to the state 5 years ago, since there are many new
debugger implementation built on the current model.

The issue #172702 has much less impact than this API solves. Issue #172702 is strictly only about enable/disable of
breakpoints. It's about one particular (breakpoint) model implementation, not about general viewmodel APIs. It was
changed back before Yarda expressed it's strong opinion about compatibility and also it can be expected that it will
affect much less clients (if any) than this API change.

I agree that it's inconsistent, but now we pay for the initial mistake and these are the trade-offs.

If someone needs the asynchronous behavior of enable/disable of breakpoints, we can easily add e.g. SELECT constant to
AsynchronousModelFilter.CALL enum.
Comment 28 ivan 2009-10-01 23:36:48 UTC
"If someone needs the asynchronous behavior of enable/disable of breakpoints, we can easily add e.g. SELECT
constant to AsynchronousModelFilter.CALL enum."

Thanks. That's a good plan to go forward with. 
Comment 29 Martin Entlicher 2009-10-02 16:22:50 UTC
O.K. I've submitted issue #173631 for that.