Cache URIs resolution.

pkg/analyzer/lib/src/dart/analysis/driver.dart

 // Copyright (c) 2016, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 import 'dart:async';
 import 'dart:collection';
 import 'dart:convert';
 
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/token.dart';
@@ skipping 108 matching lines @@
    * The set of explicitly analyzed files.
    */
   final _explicitFiles = new LinkedHashSet<String>();
 
   /**
    * The set of files that are currently scheduled for analysis.
    */
   final _filesToAnalyze = new LinkedHashSet<String>();
 
   /**
+   * The mapping of [Uri]s to the mapping of textual URIs to the [Source]
+   * that correspond in the current [_sourceFactory].

[Paul Berry, 2016/10/24 12:00:48]

I'm having trouble parsing this comment.  Consider rephrasing to something like
this:

"Cache of URI resolution.  The outer map key is the absolute URI of the
containing file.  The inner map key is the URI text of a directive contained in
that file.  The inner map value is the Source object which that URI text
resolves to."

[scheglov, 2016/10/24 17:51:34]

Done.
Thank you.

+   */
+  final _uriResolutionCache = <Uri, Map<String, Source>>{};
+
+  /**
    * The current file state.
    *
    * It maps file paths to the MD5 hash of the file content.
    */
   final _fileContentHashMap = <String, String>{};
 
   /**
    * Mapping from library URIs to the linked hash of the library.
    */
   final _linkedHashMap = <Uri, String>{};
@@ skipping 661 matching lines @@
 
     return _unit;
   }
 
   Uri get uri => source.uri;
 
   /**
    * Return the [_File] for the [uri] referenced in this file.
    */
   _File resolveUri(String uri) {
-    Source uriSource = driver._sourceFactory.resolveUri(source, uri);
+    Source uriSource = driver._uriResolutionCache
+        .putIfAbsent(this.uri, () => <String, Source>{})
+        .putIfAbsent(uri, () => driver._sourceFactory.resolveUri(source, uri));

[Paul Berry, 2016/10/24 12:00:48]

This seems like a rather minor optimization compared to other optimizations that
we haven't done yet.  I suspect that the only reason it's having a significant
effect is because changeFile() is still nuking all the internal data structures.

Assuming I'm right about that, I'd like to see us remove this optimization once
changeFile() has been changed to be incremental.  (Keeping an unnecessary
optimization around increases memory usage and cognitive burden).

Would you mind adding a TODO comment to remind us to consider removing this
optimization after changeFile() has been optimized?

[scheglov, 2016/10/24 17:51:34]

I agree about unnecessary optimizations.
Adding TODO.

     return new _File(driver, uriSource);
   }
 
   @override
   String toString() => uri.toString();
 
   /**
    * Fill the [_content] and [_contentHash] fields.
    *
    * If the [_content] field if it is still `null`, get the content from the
@@ skipping 119 matching lines @@
         }
       }
 
       appendDependencies(this);
     }
   }
 
   @override
   String toString() => uri.toString();
 }