Cache URIs resolution.

Cache URIs resolution.

TBR

R=brianwilkerson@google.com, paulberry@google.com
BUG=

Committed: https://github.com/dart-lang/sdk/commit/6ca7d24211a77276256b4bbdd451542db8ea067f

[scheglov, 2016-10-24 04:34:25]

Description was changed from

==========
Cache URIs resolution.

TBR

R=brianwilkerson@google.com, paulberry@google.com
BUG=
==========

to

==========
Cache URIs resolution.

TBR

R=brianwilkerson@google.com, paulberry@google.com
BUG=

Committed:
https://github.com/dart-lang/sdk/commit/6ca7d24211a77276256b4bbdd451542db8ea067f
==========

[scheglov, 2016-10-24 04:34:26]

Committed patchset #1 (id:1) manually as
6ca7d24211a77276256b4bbdd451542db8ea067f (presubmit successful).

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

lgtm

https://codereview.chromium.org/2440343002/diff/1/pkg/analyzer/lib/src/dart/a...
File pkg/analyzer/lib/src/dart/analysis/driver.dart (right):

https://codereview.chromium.org/2440343002/diff/1/pkg/analyzer/lib/src/dart/a...
pkg/analyzer/lib/src/dart/analysis/driver.dart:130: * that correspond in the
current [_sourceFactory].
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."

https://codereview.chromium.org/2440343002/diff/1/pkg/analyzer/lib/src/dart/a...
pkg/analyzer/lib/src/dart/analysis/driver.dart:818: .putIfAbsent(uri, () =>
driver._sourceFactory.resolveUri(source, uri));
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]

https://codereview.chromium.org/2440343002/diff/1/pkg/analyzer/lib/src/dart/a...
File pkg/analyzer/lib/src/dart/analysis/driver.dart (right):

https://codereview.chromium.org/2440343002/diff/1/pkg/analyzer/lib/src/dart/a...
pkg/analyzer/lib/src/dart/analysis/driver.dart:130: * that correspond in the
current [_sourceFactory].
On 2016/10/24 12:00:48, Paul Berry wrote:
> 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."

Done.
Thank you.

https://codereview.chromium.org/2440343002/diff/1/pkg/analyzer/lib/src/dart/a...
pkg/analyzer/lib/src/dart/analysis/driver.dart:818: .putIfAbsent(uri, () =>
driver._sourceFactory.resolveUri(source, uri));
On 2016/10/24 12:00:48, Paul Berry wrote:
> 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?

I agree about unnecessary optimizations.
Adding TODO.

[Brian Wilkerson, 2016-10-26 06:35:48]

lgtm