Bug 137930 - JavaPlugin#initializeAfterLoad should not check type history consistency
Summary: JavaPlugin#initializeAfterLoad should not check type history consistency
Status: RESOLVED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.1   Edit
Hardware: PC Windows XP
: P3 major (vote)
Target Milestone: 3.2 RC2   Edit
Assignee: JDT-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
Depends on:
Blocks:
 
Reported: 2006-04-21 09:09 EDT by Jerome Lanneluc CLA
Modified: 2006-04-27 11:25 EDT (History)
1 user (show)

See Also:


Attachments
A patch (13.24 KB, patch)
2006-04-25 19:27 EDT, Dirk Baeumer CLA
no flags Details | Diff
A slightly improved patch (12.94 KB, patch)
2006-04-26 04:35 EDT, Dirk Baeumer CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jerome Lanneluc CLA 2006-04-21 09:09:02 EDT
N20060421

JavaPlugin#initializeAfterLoad causes the Java model cache to be populated since it checks for type history consistency.

Thread [Worker-7] (Suspended (breakpoint at line 249 in JavaElement))
	CompilationUnit(JavaElement).getElementInfo(IProgressMonitor) line: 249
	CompilationUnit(JavaElement).getElementInfo() line: 235
	CompilationUnit(JavaElement).getChildren() line: 190
	CompilationUnit(JavaElement).getChildrenOfType(int) line: 204
	CompilationUnit.getTypes() line: 841
	CompilationUnit.getAllTypes() line: 562
	JavaModelUtil.findTypeInCompilationUnit(ICompilationUnit, String) line: 210
	IFileTypeInfo(TypeInfo).resolveType(IJavaSearchScope) line: 248
	OpenTypeHistory.internalCheckConsistency(IProgressMonitor) line: 279
	OpenTypeHistory.checkConsistency(IProgressMonitor) line: 224
	JavaPlugin.initializeAfterLoad(IProgressMonitor) line: 349
	InitializeAfterLoadJob$RealJob.run(IProgressMonitor) line: 36
	Worker.run() line: 58

This causes the CPU to be at 100% after startup for more than 30 seconds on my machine.

I'm not sure why the consistency check is done at this time, since 'Open Type' might be not be used at all during the session. The consistency check was not done at startup in 3.1.

A better way to check the consistency would be to do a searchAllType query, and remove the type infos that are not returned by the query. This could be done on the first 'Open type'.
Comment 1 Martin Aeschlimann CLA 2006-04-23 16:32:11 EDT
Dirk, any comments?
Comment 2 Jerome Lanneluc CLA 2006-04-24 11:22:40 EDT
Raising severerity as the perception of the JDT is very bad because of that (feels  very slow)
Comment 3 Dirk Baeumer CLA 2006-04-24 17:04:56 EDT
Some background info:

- the feature of populating the open type infrastructure in the background got
  removed for 3.1 due to switching to the all types cache in Core. Before that
  the UI populated its own cache which resulted in immediate opening of the open
  type dialog in most cases.

  JDT/UI got complains about this (because for the user this felt like a 
  regression; we basically got the 2.x behaviour back) and we decided that it 
  is so likely that the user will use the open type dialog when doing Java 
  development that it is worth doing the consistency check in the background
  on startup

Here are some option I can think of:

- we don't check the consistency at all after a restart and only update the
  histories on deltas. However this still has the potential of showing incorrect
  types since the UI can be initialized long after the core got loading hence 
  missing deltas.

- we don't persist the open type history hence we don't need to check it.

- we move the type history into JDT/Core. I think it will be much easier for
  Core to guarantee the consistency of the history than it is for UI.

- we do the check using the search engine. Jermone, the thing I don't understand
  here is why this will be so much faster. According to Martin Core is most
  worried about the class path initialization. How can the search engine tell
  that a type is still availiable without initializing the class path container
  as well ? Jerome, do you have any numbers/ideas how long 60 searches using the 
  all types search engine will take?
Comment 4 Jerome Lanneluc CLA 2006-04-25 06:52:05 EDT
(In reply to comment #3)
> According to Martin Core is most worried about the class path initialization. 
In this case, it is not the classpath initialization that is a problem, but the populating of the Java model, i.e. ICompilationUnit#getAllTypes() forces the underlying CU to be parsed and cached in the Java model. IClassFile#exists() is also called by the type history consistency check, and this causes the corresponding .jar file to be opened and the .class file to be decoded and put in the cache.

> How can the search engine tell that a type is still availiable without
> initializing the class path container as well ? 
If the classpath is not initialized when the SearchScope is created, then it will be initialized at this point. However from my findings, the classpath initialization is not what causing the type consistency check to be slow.

> Jerome, do you have any numbers/ideas how long 60 searches using
> the all types search engine will take?
Sorry I don't. However I was thinking that you would do a search all types only for the types in the history, grouping them by their first character. E.g. if your history is {IJavaElement, Foo, IMethod, JavaElement, JavaModel}, you would do 3 all type searches: one with "I*", one with "J*", and one with "F*".

If this proves to be too slow, then I agree that not doing the type consistency check would be the right thing to do, as a type being remove while JDT UI is inactive is very rare. Though JDT UI would need to be prepared to not find a type (improve the 'Cannot uniquely map' dialog).
Comment 5 Dirk Baeumer CLA 2006-04-25 19:27:24 EDT
Created attachment 39488 [details]
A patch

The patch uses timestamps on the container of the type to check if a existing checking is necessarz on startup.
Comment 6 Dirk Baeumer CLA 2006-04-25 19:28:04 EDT
Martin, can you please review the patch and decide if this is something that should go into RC2.
Comment 7 Dirk Baeumer CLA 2006-04-26 04:35:25 EDT
Created attachment 39509 [details]
A slightly improved patch
Comment 8 Martin Aeschlimann CLA 2006-04-26 09:43:56 EDT
patch looks good. released path > 20060426 so we can include it in our test pass today.
Comment 9 Martin Aeschlimann CLA 2006-04-27 09:30:27 EDT
Marking as fixed. No resolving of types in the history unless the timestamp of the underlying CU, CF or JAR has changed.

Jerome, it would be great if you could verify the performance improvements with your scenario.
Comment 10 Jerome Lanneluc CLA 2006-04-27 11:25:08 EDT
Confirmed that this improved a lot. I'm now seeing the CPU at 100% for only 12 seconds after the Workbench window came up. And profiling it, the 'Initializing Java Tooling...' job is only taking 800ms.

So thank you !