Bug 42759 - Migrate delta processor to comply to new notification scheme in 3.0
Summary: Migrate delta processor to comply to new notification scheme in 3.0
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.0   Edit
Hardware: PC Windows 2000
: P3 normal (vote)
Target Milestone: 3.0 M4   Edit
Assignee: Philipe Mulet CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-09-09 05:05 EDT by Philipe Mulet CLA
Modified: 2003-10-15 06:09 EDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Philipe Mulet CLA 2003-09-09 05:05:26 EDT
Build 3.0M3

If enabling the support for background building, the delta notifications issued 
by the platform are breaking some of the jdt/core model tests.

Design doc:
http://dev.eclipse.org/viewcvs/index.cgi/%7Echeckout%7E/platform-core-
home/documents/plan_concurrency_listeners.html


Can be enabled in the M3 build in debug mode with the following .options file:

org.eclipse.core.resources/debug=true
org.eclipse.core.resources/build/background=true
Comment 1 Philipe Mulet CLA 2003-09-15 06:42:50 EDT
The transition to the background autobuild support is not trivial from our end. 
I don't understand why others did not react stronger to this evolution, since 
it is clearly a breaking change. The problem for us isn't that the auto-build 
will run in a separate thread, but rather that the notification story is 
modified.

The Java resource listener is responsible for translating resource deltas into 
Java deltas, updating the model accordingly, and reacting to .classpath changes 
(by possibly creating markers when classpath is broken).

In order to implement this, we had to listen to the 4 kinds of events:
- PRE_DELETE
    - detect project deletions, and update model accordingly (need .classpath 
to figure the previous picture)

- PRE_AUTO_BUILD
    - detect classpath changes and reconcile them, so that from thereon both 
the Java builder and the Java model will reflect the latest classpath state

- POST_AUTO_BUILD
    - nit (reset global problem count to 0)

- POST_CHANGE
   - create and fire Java deltas, trigger indexing activity

The important thing for us is that these were happening in this very order all 
the time. 

In the new story, we are only going to get:

// in operation thread
- PRE_DELETE
- POST_CHANGE (1)

or 

// in autobuild thread
- PRE_AUTO_BUILD
- POST_AUTO_BUILD
- POST_CHANGE (2)

and these in no specific order (since both are working concurrently). Thus 
POST_CHANGE (1) may run prior to PRE_AUTO_BUILD or not. For us, when the 
classpath is changing, this is a killer. Only the PRE_AUTO_BUILD notification 
can react to it, since it is the only stage where we can create problem 
markers. It will compare the in-memory classpath and the one from disk, but if 
POST_CHANGE (1) has run already, then the in-memory classpath could have been 
refreshed already, due to some caching strategies. 

These issues can be solved by rewriting quite a bit of code to avoid these 
collisions, and completely separate the marker creation to use its own inmemory 
states, and not look at the model which may or not have been updated when the 
marker creation occurs.

Now, I cannot believe we are the only ones facing such issues: i.e. are we the 
only ones listening to various sort of events ? I think if you modified your 
story to simply notify PRE_AUTO_BUILD and POST_AUTO_BUILD in both threads, this 
would be quite fine for us. Note that you already duplicate the POST_CHANGE 
notification (and we may have an issue here as well, since we will perform 2 
updates of the model).

Also, I wonder if clients are not going to be affected by the fact that 
notifications may occur in a different thread than theirs, i.e. if I modify a 
resource, and listen to PRE_AUTO_BUILD to update something, then now, I would 
not have any guarantee about when this update has taken place.

So in brief, we are not ready for enabling this change yet.
Comment 2 Philipe Mulet CLA 2003-09-15 06:43:10 EDT
What about a regular build action ? (as opposed to background autobuild). Are 
we going the be notified in a certain order again, that we have to be aware so 
as to properly update ? Is it the old story, or another ? If this is the old 
story, then this means we have to have an implementation able to be compatible 
with it, along with the new one.

Your design note was picturing this evolution as a simple 'autobuild' in the 
background, but that isn't the case, since the build API itself isn't supposed 
to perform notifications. I guess, you perform notifications there so that 
builders can assume their notifications occur before/after they run. However, 
the actual listener is not aware of which thread it is, and doesn't know how to 
react. For the duplicate POST_CHANGE, how can my listener figure that it is 
reacting in the builder thread, vs. the operation thread ? No clue... 
Comment 3 Philipe Mulet CLA 2003-09-15 17:49:48 EDT
According to platform, there is no delta duplication. POST_CHANGE(2) will only 
contain builder changes, where POST_CHANGE(1) will only contain operation 
changes. 

It is somewhat strange that POST_CHANGE(2) contains less changes than 
POST_AUTO_BUILD which depicts operation changes though.

Investigating a full separation of markers creation and model update. 
Comment 4 Philipe Mulet CLA 2003-09-16 05:28:50 EDT
From JohnA:

In addition to our discussion this morning, I wanted to clarify some further 
possible misunderstandings.  After reviewing the implementation, I can say that 
we are definitely never broadcasting two resource change events simultaneously. 
We need to lock the tree to compute deltas, so the entire notification code is 
serialized on a lock object that blocks the entire tree against changes.  The 
series of events in the builder thread is also atomic:  During the PRE_BUILD / 
build / POST_BUILD sequence, there are no event broadcasts in other threads.  
Since it can be reduced to a serial sequence, this is what you should be seeing:

POST_CHANGE (1) (in operation thread)
(possibly several more POST_CHANGE operations in operation thread before 
builder thread gets a chance to run)
PRE_AUTO_BUILD
(possibly a build, but it could be pre-empted if another thread tries to modify 
the workspace)
POST_AUTO_BUILD
POST_CHANGE (2)

Where all POST_CHANGE events contain disjoint sets of changes. The auto build 
events will contain deltas that may possibly represent amalgamations of several 
prior POST_CHANGE events.
Comment 5 Philipe Mulet CLA 2003-09-17 09:21:00 EDT
Migration almost complete and working. 
Comment 6 Philipe Mulet CLA 2003-09-19 07:08:54 EDT
Released for integration.
Comment 7 David Audel CLA 2003-10-15 06:09:58 EDT
Verified.