Cache model elements for compiler elements.

Cache model elements for compiler elements.

With this Editor Core tests run 2 times faster.
Indexing is now 3 times faster!


R=brianwilkerson@google.com
BUG=
TEST=

Committed: https://code.google.com/p/dart/source/detail?r=9635

[Brian Wilkerson, 2012-07-13 15:00:08]

LGTM, after a couple of minor changes. Looking forward to the performance boost!

https://chromiumcodereview.appspot.com/10692198/diff/3001/editor/tools/plugin...
File
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/index/util/ElementFactory.java
(right):

https://chromiumcodereview.appspot.com/10692198/diff/3001/editor/tools/plugin...
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/index/util/ElementFactory.java:106:
Element result = compilerToModelElement.get(element);
This looks wrong to me. Each of the more specific getElement methods already
handles the caching of elements, so I don't think we need to add that logic
here, but more importantly, I think it will do the wrong thing with
FieldElements. If I understand correctly, field elements are not supposed to be
cached in the compilerToModelElement map, but this breaks that. I think you need
to revert this method.

https://chromiumcodereview.appspot.com/10692198/diff/3001/editor/tools/plugin...
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/index/util/ElementFactory.java:111:
result = getElement((FieldElement) element);
nit: this could be getElement((FieldElement) element, false, false) and avoid
one extra method call.

https://chromiumcodereview.appspot.com/10692198/diff/3001/editor/tools/plugin...
File
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/model/DartLibraryImpl.java
(right):

https://chromiumcodereview.appspot.com/10692198/diff/3001/editor/tools/plugin...
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/model/DartLibraryImpl.java:95:
public static void clearLocalUrisCache() {
This makes me nervous because it's too easy for someone to forget or not realize
that this needs to be invoked when changing the project structure. In a
follow-on CL I'd like to see this made more fail-proof. Perhaps we could tie
this in to the DeltaProcessor or some other resource-change based mechanism so
that it can't be overlooked.

[scheglov, 2012-07-13 16:48:09]

https://chromiumcodereview.appspot.com/10692198/diff/3001/editor/tools/plugin...
File
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/index/util/ElementFactory.java
(right):

https://chromiumcodereview.appspot.com/10692198/diff/3001/editor/tools/plugin...
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/index/util/ElementFactory.java:106:
Element result = compilerToModelElement.get(element);
On 2012/07/13 15:00:08, Brian Wilkerson wrote:
> This looks wrong to me. Each of the more specific getElement methods already
> handles the caching of elements, so I don't think we need to add that logic
> here, but more importantly, I think it will do the wrong thing with
> FieldElements. If I understand correctly, field elements are not supposed to
be
> cached in the compilerToModelElement map, but this breaks that. I think you
need
> to revert this method.

Done.

Well, actually for FieldElement we would cache only case "false, false" and use
also only here and only for this case. So, not wrong, but not nice.

https://chromiumcodereview.appspot.com/10692198/diff/3001/editor/tools/plugin...
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/index/util/ElementFactory.java:111:
result = getElement((FieldElement) element);
On 2012/07/13 15:00:08, Brian Wilkerson wrote:
> nit: this could be getElement((FieldElement) element, false, false) and avoid
> one extra method call.

Done.

https://chromiumcodereview.appspot.com/10692198/diff/3001/editor/tools/plugin...
File
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/model/DartLibraryImpl.java
(right):

https://chromiumcodereview.appspot.com/10692198/diff/3001/editor/tools/plugin...
editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/model/DartLibraryImpl.java:95:
public static void clearLocalUrisCache() {
On 2012/07/13 15:00:08, Brian Wilkerson wrote:
> This makes me nervous because it's too easy for someone to forget or not
realize
> that this needs to be invoked when changing the project structure. In a
> follow-on CL I'd like to see this made more fail-proof. Perhaps we could tie
> this in to the DeltaProcessor or some other resource-change based mechanism so
> that it can't be overlooked.

OK, I will try to address this in the next CL.