Bug 526520 - ASTParser: better reusability
Summary: ASTParser: better reusability
Status: NEW
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 4.7.1a   Edit
Hardware: PC Mac OS X
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: JDT-Core-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
: 392333 (view as bug list)
Depends on:
Blocks: 392225
  Show dependency tree
 
Reported: 2017-10-26 11:02 EDT by Christian Dietrich CLA
Modified: 2020-06-05 18:36 EDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Dietrich CLA 2017-10-26 11:02:08 EDT
At Xtext we use ASTParser to resolve Bindings for ITypes. We basically use a shared ASTParser instance and call parser.resolveBindings over and over again.

I am not sure if this is a proper way of reusing and how much of the stuff is actually reused or basically the same work is done over and over again.

Here is what we do:

	private IBinding resolveBindings(IType jdtType, IJavaProject javaProject) {
		ThreadLocal<Boolean> abortOnMissingSource = JavaModelManager.getJavaModelManager().abortOnMissingSource;
		Boolean wasAbortOnMissingSource = abortOnMissingSource.get();
		try {
			abortOnMissingSource.set(Boolean.TRUE);
			resolveBinding.start();
	
			parser.setWorkingCopyOwner(workingCopyOwner);
			parser.setIgnoreMethodBodies(true);
			
			parser.setProject(javaProject);
			
			Map<String, String> options = javaProject.getOptions(true);
			
			options.put(JavaCore.COMPILER_DOC_COMMENT_SUPPORT, JavaCore.DISABLED);
			parser.setCompilerOptions(options);
	
			IBinding[] bindings = parser.createBindings(new IJavaElement[] { jdtType }, null);
			resolveBinding.stop();
			return bindings[0];
		} finally {
			abortOnMissingSource.set(wasAbortOnMissingSource);
		}
	}

i am not sure this is the best way to do since we call this method about 270 times even for a very small xtext project. 


There might be other secenarios where user like to reuse ASTParsers as well e.g. to "mimic" a kind of incremental build by keeping all the stuff and just reparse a single changed file (i discussed this with Martin Lippert and Stephan Herrmann at EclipseCon)
Comment 1 Stephan Herrmann CLA 2017-10-31 16:36:14 EDT
The (internal) problem here is: all state (compiler & INameEnvironment) is held in the internal class CompilationUnitResolver, which is used by ASTParser only via static methods, i.e., any state is lost at the end of createBindings() or similar.

From a client's p.o.v. I could imagine a pair of methods in ASTParser:
  startSession();
  stopSession();

startSession() can be called after all ASTParser.set*(), to the effect that an instance of CompilationUnitResolver is prepared, and kept for repeated use.

After that, any further call to ASTParser.set*() is illegal and will throw an exception (regarding the instance on which startSession() was invoked).

Finally, stopSession() will release the CompilationUnitResolver and ASTParser.set*() will be admitted again. Alternatively, we could simply require that a new ASTParser be created (so no stopSession() would be needed).

Still need to figure out, how much effort it would be, to craft a re-usable CompilationUnitResolver. After the first invocation, CUR#accept() might be our friend (replacing beginToCompile()).

Perhaps, the new functionality should go into a subclass SessionBasedASTParser.

Setting a reasonable target, but no promise yet.
Comment 2 Martin Lippert CLA 2019-08-07 04:29:16 EDT
I am not an expert here, but what happens to the environment if you would pass in changed source files within such a session and resolve node again? Isn't the necessary information somehow stored in the environment, so that we would need some cleanup in between different parse attempts?
Comment 3 Stephan Herrmann CLA 2019-10-25 08:09:09 EDT
Wow, genie just unearthed an old related idea: bug 150657

At a first glance I like the idea of explicitly exposing this new kind of environment.

Any comment on whether the old proposal would help here?
Comment 4 Stephan Herrmann CLA 2019-12-10 10:26:26 EST
*** Bug 392333 has been marked as a duplicate of this bug. ***