Cache the current working directory.

Cache the current working directory.

Recomputing this isn't terribly expensive, but it happens a lot so it's
nice to avoid.

Also avoid calling it in relative() when the path is already relative.

This brings current from about 0.070 iterations/us to about 0.097, and relative from about 0.080 to about 1.589.

R=rnystrom@google.com

Committed: https://github.com/dart-lang/path/commit/414bb1e1b7928090b88386c5f15f8ddee43fe96b

[Bob Nystrom, 2015-11-24 17:25:16]

Benchmark? Numbers?

https://codereview.chromium.org/1473563003/diff/1/lib/path.dart
File lib/path.dart (right):

https://codereview.chromium.org/1473563003/diff/1/lib/path.dart#newcode82
lib/path.dart:82: if (uri == _oldUriBase) return _oldCurrent;
Document that reparsing the base URI is slow so it's being cached here.

https://codereview.chromium.org/1473563003/diff/1/lib/path.dart#newcode107
lib/path.dart:107: String _oldCurrent;
Instead of "old", how about "_currentUriBase" and "_current"?

[nweiz, 2015-11-30 22:46:56]

Code review changes

[nweiz, 2015-11-30 22:51:44]

Description was changed from

==========
Cache the current working directory.

Recomputing this isn't terribly expensive, but it happens a lot so it's
nice to avoid.

Also avoid calling it in relative() when the path is already relative.

R=rnystrom@google.com
==========

to

==========
Cache the current working directory.

Recomputing this isn't terribly expensive, but it happens a lot so it's
nice to avoid.

Also avoid calling it in relative() when the path is already relative.

This brings current from about 0.070 iterations/us to about 0.097, and relative
from about 0.080 to about 1.589.

R=rnystrom@google.com
==========

[nweiz, 2015-11-30 22:52:10]

> Benchmark? Numbers?

Done.

https://codereview.chromium.org/1473563003/diff/1/lib/path.dart
File lib/path.dart (right):

https://codereview.chromium.org/1473563003/diff/1/lib/path.dart#newcode82
lib/path.dart:82: if (uri == _oldUriBase) return _oldCurrent;
On 2015/11/24 17:25:16, Bob Nystrom wrote:
> Document that reparsing the base URI is slow so it's being cached here.

Done.

https://codereview.chromium.org/1473563003/diff/1/lib/path.dart#newcode107
lib/path.dart:107: String _oldCurrent;
On 2015/11/24 17:25:16, Bob Nystrom wrote:
> Instead of "old", how about "_currentUriBase" and "_current"?

Done.

[nweiz, 2015-11-30 22:53:12]

Description was changed from

==========
Cache the current working directory.

Recomputing this isn't terribly expensive, but it happens a lot so it's
nice to avoid.

Also avoid calling it in relative() when the path is already relative.

This brings current from about 0.070 iterations/us to about 0.097, and relative
from about 0.080 to about 1.589.

R=rnystrom@google.com
==========

to

==========
Cache the current working directory.

Recomputing this isn't terribly expensive, but it happens a lot so it's
nice to avoid.

Also avoid calling it in relative() when the path is already relative.

This brings current from about 0.070 iterations/us to about 0.097, and relative
from about 0.080 to about 1.589.

R=rnystrom@google.com

Committed:
https://github.com/dart-lang/path/commit/414bb1e1b7928090b88386c5f15f8ddee43f...
==========

[nweiz, 2015-11-30 22:53:13]

Committed patchset #2 (id:20001) manually as
414bb1e1b7928090b88386c5f15f8ddee43fe96b (presubmit successful).

[Bob Nystrom, 2015-11-30 23:02:25]

lgtm

[Lasse Reichstein Nielsen, 2015-12-01 06:52:58]

lrn@google.com changed reviewers:
+ lrn@google.com

[Lasse Reichstein Nielsen, 2015-12-01 06:52:58]

https://codereview.chromium.org/1473563003/diff/20001/lib/path.dart
File lib/path.dart (right):

https://codereview.chromium.org/1473563003/diff/20001/lib/path.dart#newcode89
lib/path.dart:89: _current = uri.resolve('.').toString();
That is likely to be slower than necessary.
Instead maybe:
   var path = uri.path;
   if (path.length > 1) {
     uri = uri.replace(path: uri.path.substring(0, path.length - 1));
   }
  _current = uri.toString)(;
(That is likely to be faster, but not guaranteed, since you still reparse the
path, but you avoid the complexity of dot-normalizing a path).

[Lasse Reichstein Nielsen, 2015-12-01 06:53:54]

https://codereview.chromium.org/1473563003/diff/20001/lib/path.dart
File lib/path.dart (right):

https://codereview.chromium.org/1473563003/diff/20001/lib/path.dart#newcode89
lib/path.dart:89: _current = uri.resolve('.').toString();
(That does assume that Uri.base has no query or fragment, so maybe it's not
general enough).

[nweiz, 2015-12-01 20:50:46]

https://codereview.chromium.org/1473563003/diff/20001/lib/path.dart
File lib/path.dart (right):

https://codereview.chromium.org/1473563003/diff/20001/lib/path.dart#newcode89
lib/path.dart:89: _current = uri.resolve('.').toString();
On 2015/12/01 06:53:54, Lasse Reichstein Nielsen wrote:
> (That does assume that Uri.base has no query or fragment, so maybe it's not
> general enough).

Given that this is cached now, I think it's probably not worth putting a lot of
effort into optimizing the code to compute the cached value.