Bug 180107 - [PERF] need CompilationParticipant.buildComplete() API
Summary: [PERF] need CompilationParticipant.buildComplete() API
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.4 M3   Edit
Assignee: Kent Johnson CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-03-29 21:27 EDT by Walter Harley CLA
Modified: 2007-10-29 05:09 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Walter Harley CLA 2007-03-29 21:27:49 EDT
The CompilationParticipant API lets compilation participants know when a build is starting and when annotations need to be processed.  There is no indication when a build is complete; processAnnotations() can be called repeatedly within the context of a single build.

We would like a buildComplete() method to be added.  This will let the compilation participant free up objects that are needed for the duration of the build but not afterwards.  In particular, because of bug 175794 the APT compilation participant needs to hang on to lists of CategorizedProblems, which can grow quite large in a large project.

Adding this method will not break any existing clients, because CompilationParticipant is a class rather than an interface.
Comment 1 Philipe Mulet CLA 2007-03-30 05:37:07 EDT
Remember that API freeze was M6.
Comment 2 Philipe Mulet CLA 2007-03-30 05:44:27 EDT
Just for my education.
Why would a participant store data into itself to achieve this ? The lifecycle of a participant is typically longer. Why not recording the data elsewhere, for some object with a shorter lifespan ?
Comment 3 Walter Harley CLA 2007-03-30 12:04:59 EDT
> Remember that API freeze was M6

Yes, and M7 is about polish and performance improvement.  This is a case where getting a performance improvement requires a (non-breaking) API change.


> Why not recording the data elsewhere, for some object with a shorter lifespan?

Within the implementation of the participant, of course it makes sense to manage the data within some shorter-lived object.  But how does the lifespan of that object get managed?

Bottom line is we have some data that can be discarded as soon as a build is complete; but we currently have no way of knowing that the build is finished, so we have no way of discarding it.
Comment 4 Philipe Mulet CLA 2007-04-16 21:50:33 EDT
Don't you have a resource change listener already ? Listening to a POST_AUTO_BUILD event to clear the data would do it.

Or we can broadcast as requested here, but if you have a listener already, this would be the simplest for now.
Comment 5 Philipe Mulet CLA 2007-04-16 21:52:35 EDT
Now, the #buildFinished() addition would be consistent on CompilationParticipant with existing API (#buildStarting(...)).

Kent - if going for API addition, pls send a request to the PMC list.
Comment 6 Walter Harley CLA 2007-04-16 22:53:48 EDT
I didn't realize that POST_AUTO_BUILD would do it.  We do have a resource listener but I am not sure it is in quite the right place.  But even if it is not, we could easily create one - unlike the buildStarting case, registering this listener in time is simple since we know we have already been called.

Does the POST_AUTO_BUILD come on the same thread as buildStarting()?  Or, more importantly, is it guaranteed to come before the next buildStarting()?  It would be bad to discard resources midway through the next build...

If the threading is okay, then perhaps all that is needed, for now, is to add a comment on CompilationParticipant.buildStarting(), to say "in order to be notified when the build completes, use a ResourceChangeListener to listen for POST_AUTO_BUILD events."

I do think that it would be convenient to have it all in one API, but if the threading of the resource change event is okay then this is something that could happen after 3.3.
Comment 7 Philipe Mulet CLA 2007-04-17 08:49:13 EDT
I think the threading issue will be no different (simply when we get notified, we notify you through the new API, or you listen to the same notification we listen to). So the API addition is more of a convenience than a must have.

Comment 8 Kent Johnson CLA 2007-04-17 11:41:09 EDT
Take a look at the 2 methods on the JavaBuilder -> buildStarting() and buildFinished().

These are called from the DeltaProcessor, who is listening for IResourceChangeEvent.PRE_BUILD and IResourceChangeEvent.POST_BUILD.

We should update the spec of CompilationParticipant.buildStarting() to inform users that they can also create a listener to handle these tasks, instead of adding a new API method.

Any objections?
Comment 9 Philipe Mulet CLA 2007-04-17 22:24:37 EDT
A fair reason to add the API is for simplicity, since contributor already registered a comp. participant. Adding an extra rsc change listener just for this notification feels overkill. But this is late into the game...
Comment 10 Walter Harley CLA 2007-05-14 02:30:09 EDT
I've just implemented the functionality with a resource change listener for APT.  Based on that experience, I'd say we should definitely add this API (but not for 3.3).

Reason is that the aboutToBuild() method takes an IJavaProject.  To be parallel (i.e., to let participants discard resources specific to a project), we need buildComplete(IJavaProject).  Implementing this with a resource change listener requires using a resource delta visitor to identify the projects that were built; not complex, but a bit more involved than necessary, and easy to get wrong since the POST_BUILD event source is often the Workspace even if only one project was built.  There's no reason that every client should have to write the same code.
Comment 11 Kent Johnson CLA 2007-10-17 15:16:03 EDT
Released into HEAD for 3.4M3


/**
 * Notifies this participant that a build has finished for the project.
 * This will be sent, even if buildStarting() was not sent when no source
 * files needed to be compiled or the build failed.
 * Only sent to participants interested in the project.
 * @param project the project about to build
 * @since 3.4
  */
public void buildFinished(IJavaProject project) {
	// do nothing by default
}
Comment 12 Frederic Fusier CLA 2007-10-29 05:09:06 EDT
Verified for 3.4M3 using I20071029-0010 build.