Bug 91213 - [performance] DOM name binding resolution should be done only when required
Summary: [performance] DOM name binding resolution should be done only when required
Status: RESOLVED WONTFIX
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.1   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: JDT-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
Depends on:
Blocks:
 
Reported: 2005-04-12 17:14 EDT by Olivier Thomann CLA
Modified: 2005-04-28 13:43 EDT (History)
1 user (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Olivier Thomann CLA 2005-04-12 17:14:56 EDT 
Inside an DOMVisitor, the request to resolve the binding for a name should be
done only when required.
If you have some code that starts with:

import org.eclipse.swt.graphics.Point;
import org.eclipse.swt.graphics.Rectangle;
import org.eclipse.swt.widgets.Composite;
import org.eclipse.swt.widgets.Control;
import org.eclipse.swt.widgets.Shell;

import org.eclipse.jface.dialogs.IDialogConstants;
import org.eclipse.jface.dialogs.IDialogSettings;
import org.eclipse.jface.operation.IRunnableContext;

import org.eclipse.ui.PlatformUI;

import org.eclipse.jdt.core.search.IJavaSearchScope;

import org.eclipse.jdt.internal.ui.IJavaHelpContextIds;
import org.eclipse.jdt.internal.ui.JavaPlugin;
...

and you have a visitor that does this:

	public boolean visit(SimpleName node) {
		Binding binding = node.resolveBinding();
                ....  
                return false;
	}

or in the endVisit method, this will trigger lot of binding resolution for node
that you don't need. For example, in:
import org.eclipse.jface.dialogs.IDialogConstants;

you would end up resolving:
org
org.eclipse
eclipse
org.eclipse.jface
jface
etc.

You end up resolving 9 bindings when I think only the binding for the import
reference is used afterwards.
Comment 1 Dirk Baeumer CLA 2005-04-13 05:04:54 EDT 
We can check the obivious cases here but recoding this in general for 3.1 isn't
an option. This would invalidate to much code so late in the cycle. 

Olivier, why do we resolve org.eclipse & org.eclipse.jface. There aren't simply
names. And can you please enumerate what the impact is of doing this (speed &
performance). We always had the impression that resolving these bindings is
cheap since we request the bindings when we build the AST.

CC Dani since this might affect editor (sematic highlighting & mark occurrences)
Comment 2 Dirk Baeumer CLA 2005-04-13 05:28:59 EDT 
One way to fix this is to avoid iterating over import declarations. This will
save lots of accesses and will not impact most of the algorithms.
Comment 3 Dirk Baeumer CLA 2005-04-13 07:06:22 EDT 
I did some benchmarking with SEF (which access the binding of all simple names
in the CU) and the visist(...) method doesn't show up in the traces for large
files (e.g.StyledText) or medium file.

Another idea to speed this up (if there is a need at all) a little bit is to do
some name comparison upfront. For example in SEF we could check if the name of
the node == field name.

Other refactorings (extract method, flow analyzer) are visiting simple names but
only in the scope of a method.
Comment 4 Dirk Baeumer CLA 2005-04-28 13:43:59 EDT 
No action planned for 3.1 here. The call doesn't show up in performance traces
and according to Philippe the bindings are already exist in the compiler.