Skip to content

Commit

Permalink
Cache packageName to base Uri mapping.
Browse files Browse the repository at this point in the history
This makes analysis with using incremental analysis cache 30% faster.
Without this change getBase() takes 40% (!) of total analysis time.
With this change - just 0.79% of total time.

Although the remaining 17% of PackagesBase.resolve() make my cry.
Why URI manipulations are SO SLOW?!

R=brianwilkerson@google.com, pquitslund@google.com, kevmoo@google.com
BUG=

Review URL: https://codereview.chromium.org/2041103005 .
  • Loading branch information
scheglov committed Jun 8, 2016
1 parent 1b9b3af commit 71ec483
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 3 deletions.
9 changes: 7 additions & 2 deletions lib/src/packages_io_impl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,15 @@ import "packages_impl.dart";
/// A [Packages] implementation based on a local directory.
class FilePackagesDirectoryPackages extends PackagesBase {
final Directory _packageDir;
final Map<String, Uri> _packageToBaseUriMap = <String, Uri>{};

FilePackagesDirectoryPackages(this._packageDir);

Uri getBase(String packageName) =>
new Uri.file(path.join(_packageDir.path, packageName, '.'));
Uri getBase(String packageName) {
return _packageToBaseUriMap.putIfAbsent(packageName, () {
return new Uri.file(path.join(_packageDir.path, packageName, '.'));
});
}

Iterable<String> _listPackageNames() {
return _packageDir
Expand Down
2 changes: 1 addition & 1 deletion pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: package_config
version: 0.1.4
version: 0.1.5
description: Support for working with Package Resolution config files.
author: Dart Team <misc@dartlang.org>
homepage: https://github.com/dart-lang/package_config
Expand Down

2 comments on commit 71ec483

@lrhn
Copy link
Member

@lrhn lrhn commented on 71ec483 Jun 9, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

URL manipulation is slow in part because it's a implementing the RFC 3986 resolution algorithm which isn't exactly designed for speed. Most path resolutions won't need the full generality, and I want to see if we can do something faster for the simple cases.

And in part because we have only one implementation which does a lot of string manipulation and copying. - we should have a special-case version for URIs that don't need any normalization and where we can just store the original string passed to Uri.parse.

So, basically, because we make too many intermediate strings.

@scheglov
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's what I thought.
We had to add FastUri implementation into analyzer.
This allowed us to make analysis much faster, 25% at that time.

Please sign in to comment.