Skip to content

Commit b97f26f

Browse files
nex3nex3
authored
Add a per-importer cache for loads that aren't cacheable en masse (#2219)
Inspired by comments on #2215
1 parent 2a9eaad commit b97f26f

File tree

4 files changed

+133
-71
lines changed

4 files changed

+133
-71
lines changed

‎lib/src/async_import_cache.dart‎

Lines changed: 58 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import 'importer/no_op.dart';
1616
import 'importer/utils.dart';
1717
import 'io.dart';
1818
import 'logger.dart';
19+
import 'util/map.dart';
1920
import 'util/nullable.dart';
2021
import 'utils.dart';
2122

@@ -44,30 +45,28 @@ final class AsyncImportCache {
4445
/// The `forImport` in each key is true when this canonicalization is for an
4546
/// `@import` rule. Otherwise, it's for a `@use` or `@forward` rule.
4647
///
47-
/// This cache isn't used for relative imports, because they depend on the
48-
/// specific base importer. That's stored separately in
49-
/// [_relativeCanonicalizeCache].
48+
/// This cache covers loads that go through the entire chain of [_importers],
49+
/// but it doesn't cover individual loads or loads in which any importer
50+
/// accesses `containingUrl`. See also [_perImporterCanonicalizeCache].
5051
final _canonicalizeCache =
5152
<(Uri, {bool forImport}), AsyncCanonicalizeResult?>{};
5253

53-
/// The canonicalized URLs for each non-canonical URL that's resolved using a
54-
/// relative importer.
54+
/// Like [_canonicalizeCache] but also includes the specific importer in the
55+
/// key.
5556
///
56-
/// The map's keys have four parts:
57+
/// This is used to cache both relative imports from the base importer and
58+
/// individual importer results in the case where some other component of the
59+
/// importer chain isn't cacheable.
60+
final _perImporterCanonicalizeCache =
61+
<(AsyncImporter, Uri, {bool forImport}), AsyncCanonicalizeResult?>{};
62+
63+
/// A map from the keys in [_perImporterCanonicalizeCache] that are generated
64+
/// for relative URL loads agains the base importer to the original relative
65+
/// URLs what were loaded.
5766
///
58-
/// 1. The URL passed to [canonicalize] (the same as in [_canonicalizeCache]).
59-
/// 2. Whether the canonicalization is for an `@import` rule.
60-
/// 3. The `baseImporter` passed to [canonicalize].
61-
/// 4. The `baseUrl` passed to [canonicalize].
62-
///
63-
/// The map's values are the same as the return value of [canonicalize].
64-
final _relativeCanonicalizeCache = <(
65-
Uri, {
66-
bool forImport,
67-
AsyncImporter baseImporter,
68-
Uri? baseUrl
69-
}),
70-
AsyncCanonicalizeResult?>{};
67+
/// This is used to invalidate the cache when files are changed.
68+
final _nonCanonicalRelativeUrls =
69+
<(AsyncImporter, Uri, {bool forImport}), Uri>{};
7170

7271
/// The parsed stylesheets for each canonicalized import URL.
7372
final _importCache = <Uri, Stylesheet?>{};
@@ -155,18 +154,17 @@ final class AsyncImportCache {
155154
}
156155

157156
if (baseImporter != null && url.scheme == '') {
158-
var relativeResult = await putIfAbsentAsync(_relativeCanonicalizeCache, (
159-
url,
160-
forImport: forImport,
161-
baseImporter: baseImporter,
162-
baseUrl: baseUrl
163-
), () async {
164-
var (result, cacheable) = await _canonicalize(
165-
baseImporter, baseUrl?.resolveUri(url) ?? url, baseUrl, forImport);
157+
var resolvedUrl = baseUrl?.resolveUri(url) ?? url;
158+
var key = (baseImporter, resolvedUrl, forImport: forImport);
159+
var relativeResult =
160+
await putIfAbsentAsync(_perImporterCanonicalizeCache, key, () async {
161+
var (result, cacheable) =
162+
await _canonicalize(baseImporter, resolvedUrl, baseUrl, forImport);
166163
assert(
167164
cacheable,
168165
"Relative loads should always be cacheable because they never "
169166
"provide access to the containing URL.");
167+
if (baseUrl != null) _nonCanonicalRelativeUrls[key] = url;
170168
return result;
171169
});
172170
if (relativeResult != null) return relativeResult;
@@ -182,17 +180,41 @@ final class AsyncImportCache {
182180
// `canonicalize()` calls we've attempted are cacheable. Only if they are do
183181
// we store the result in the cache.
184182
var cacheable = true;
185-
for (var importer in _importers) {
183+
for (var i = 0; i < _importers.length; i++) {
184+
var importer = _importers[i];
185+
var perImporterKey = (importer, url, forImport: forImport);
186+
switch (_perImporterCanonicalizeCache.getOption(perImporterKey)) {
187+
case (var result?,):
188+
return result;
189+
case (null,):
190+
continue;
191+
}
192+
186193
switch (await _canonicalize(importer, url, baseUrl, forImport)) {
187194
case (var result?, true) when cacheable:
188195
_canonicalizeCache[key] = result;
189196
return result;
190197

191-
case (var result?, _):
192-
return result;
193-
194-
case (_, false):
195-
cacheable = false;
198+
case (var result, true) when !cacheable:
199+
_perImporterCanonicalizeCache[perImporterKey] = result;
200+
if (result != null) return result;
201+
202+
case (var result, false):
203+
if (cacheable) {
204+
// If this is the first uncacheable result, add all previous results
205+
// to the per-importer cache so we don't have to re-run them for
206+
// future uses of this importer.
207+
for (var j = 0; j < i; j++) {
208+
_perImporterCanonicalizeCache[(
209+
_importers[j],
210+
url,
211+
forImport: forImport
212+
)] = null;
213+
}
214+
cacheable = false;
215+
}
216+
217+
if (result != null) return result;
196218
}
197219
}
198220

@@ -315,7 +337,7 @@ final class AsyncImportCache {
315337
Uri sourceMapUrl(Uri canonicalUrl) =>
316338
_resultsCache[canonicalUrl]?.sourceMapUrl ?? canonicalUrl;
317339

318-
/// Clears the cached canonical version of the given [url].
340+
/// Clears the cached canonical version of the given non-canonical [url].
319341
///
320342
/// Has no effect if the canonical version of [url] has not been cached.
321343
///
@@ -324,7 +346,8 @@ final class AsyncImportCache {
324346
void clearCanonicalize(Uri url) {
325347
_canonicalizeCache.remove((url, forImport: false));
326348
_canonicalizeCache.remove((url, forImport: true));
327-
_relativeCanonicalizeCache.removeWhere((key, _) => key.$1 == url);
349+
_perImporterCanonicalizeCache.removeWhere(
350+
(key, _) => key.$2 == url || _nonCanonicalRelativeUrls[key] == url);
328351
}
329352

330353
/// Clears the cached parse tree for the stylesheet with the given

‎lib/src/import_cache.dart‎

Lines changed: 57 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
// DO NOT EDIT. This file was generated from async_import_cache.dart.
66
// See tool/grind/synchronize.dart for details.
77
//
8-
// Checksum: 36bc42050cf2eb3a43f36376c4f06c1708eee777
8+
// Checksum: 4362e28e5cd425786c235d2a6a2bb60539403799
99
//
1010
// ignore_for_file: unused_import
1111

@@ -23,6 +23,7 @@ import 'importer/no_op.dart';
2323
import 'importer/utils.dart';
2424
import 'io.dart';
2525
import 'logger.dart';
26+
import 'util/map.dart';
2627
import 'util/nullable.dart';
2728
import 'utils.dart';
2829

@@ -47,29 +48,26 @@ final class ImportCache {
4748
/// The `forImport` in each key is true when this canonicalization is for an
4849
/// `@import` rule. Otherwise, it's for a `@use` or `@forward` rule.
4950
///
50-
/// This cache isn't used for relative imports, because they depend on the
51-
/// specific base importer. That's stored separately in
52-
/// [_relativeCanonicalizeCache].
51+
/// This cache covers loads that go through the entire chain of [_importers],
52+
/// but it doesn't cover individual loads or loads in which any importer
53+
/// accesses `containingUrl`. See also [_perImporterCanonicalizeCache].
5354
final _canonicalizeCache = <(Uri, {bool forImport}), CanonicalizeResult?>{};
5455

55-
/// The canonicalized URLs for each non-canonical URL that's resolved using a
56-
/// relative importer.
56+
/// Like [_canonicalizeCache] but also includes the specific importer in the
57+
/// key.
5758
///
58-
/// The map's keys have four parts:
59+
/// This is used to cache both relative imports from the base importer and
60+
/// individual importer results in the case where some other component of the
61+
/// importer chain isn't cacheable.
62+
final _perImporterCanonicalizeCache =
63+
<(Importer, Uri, {bool forImport}), CanonicalizeResult?>{};
64+
65+
/// A map from the keys in [_perImporterCanonicalizeCache] that are generated
66+
/// for relative URL loads agains the base importer to the original relative
67+
/// URLs what were loaded.
5968
///
60-
/// 1. The URL passed to [canonicalize] (the same as in [_canonicalizeCache]).
61-
/// 2. Whether the canonicalization is for an `@import` rule.
62-
/// 3. The `baseImporter` passed to [canonicalize].
63-
/// 4. The `baseUrl` passed to [canonicalize].
64-
///
65-
/// The map's values are the same as the return value of [canonicalize].
66-
final _relativeCanonicalizeCache = <(
67-
Uri, {
68-
bool forImport,
69-
Importer baseImporter,
70-
Uri? baseUrl
71-
}),
72-
CanonicalizeResult?>{};
69+
/// This is used to invalidate the cache when files are changed.
70+
final _nonCanonicalRelativeUrls = <(Importer, Uri, {bool forImport}), Uri>{};
7371

7472
/// The parsed stylesheets for each canonicalized import URL.
7573
final _importCache = <Uri, Stylesheet?>{};
@@ -155,18 +153,16 @@ final class ImportCache {
155153
}
156154

157155
if (baseImporter != null && url.scheme == '') {
158-
var relativeResult = _relativeCanonicalizeCache.putIfAbsent((
159-
url,
160-
forImport: forImport,
161-
baseImporter: baseImporter,
162-
baseUrl: baseUrl
163-
), () {
164-
var (result, cacheable) = _canonicalize(
165-
baseImporter, baseUrl?.resolveUri(url) ?? url, baseUrl, forImport);
156+
var resolvedUrl = baseUrl?.resolveUri(url) ?? url;
157+
var key = (baseImporter, resolvedUrl, forImport: forImport);
158+
var relativeResult = _perImporterCanonicalizeCache.putIfAbsent(key, () {
159+
var (result, cacheable) =
160+
_canonicalize(baseImporter, resolvedUrl, baseUrl, forImport);
166161
assert(
167162
cacheable,
168163
"Relative loads should always be cacheable because they never "
169164
"provide access to the containing URL.");
165+
if (baseUrl != null) _nonCanonicalRelativeUrls[key] = url;
170166
return result;
171167
});
172168
if (relativeResult != null) return relativeResult;
@@ -182,17 +178,41 @@ final class ImportCache {
182178
// `canonicalize()` calls we've attempted are cacheable. Only if they are do
183179
// we store the result in the cache.
184180
var cacheable = true;
185-
for (var importer in _importers) {
181+
for (var i = 0; i < _importers.length; i++) {
182+
var importer = _importers[i];
183+
var perImporterKey = (importer, url, forImport: forImport);
184+
switch (_perImporterCanonicalizeCache.getOption(perImporterKey)) {
185+
case (var result?,):
186+
return result;
187+
case (null,):
188+
continue;
189+
}
190+
186191
switch (_canonicalize(importer, url, baseUrl, forImport)) {
187192
case (var result?, true) when cacheable:
188193
_canonicalizeCache[key] = result;
189194
return result;
190195

191-
case (var result?, _):
192-
return result;
193-
194-
case (_, false):
195-
cacheable = false;
196+
case (var result, true) when !cacheable:
197+
_perImporterCanonicalizeCache[perImporterKey] = result;
198+
if (result != null) return result;
199+
200+
case (var result, false):
201+
if (cacheable) {
202+
// If this is the first uncacheable result, add all previous results
203+
// to the per-importer cache so we don't have to re-run them for
204+
// future uses of this importer.
205+
for (var j = 0; j < i; j++) {
206+
_perImporterCanonicalizeCache[(
207+
_importers[j],
208+
url,
209+
forImport: forImport
210+
)] = null;
211+
}
212+
cacheable = false;
213+
}
214+
215+
if (result != null) return result;
196216
}
197217
}
198218

@@ -312,7 +332,7 @@ final class ImportCache {
312332
Uri sourceMapUrl(Uri canonicalUrl) =>
313333
_resultsCache[canonicalUrl]?.sourceMapUrl ?? canonicalUrl;
314334

315-
/// Clears the cached canonical version of the given [url].
335+
/// Clears the cached canonical version of the given non-canonical [url].
316336
///
317337
/// Has no effect if the canonical version of [url] has not been cached.
318338
///
@@ -321,7 +341,8 @@ final class ImportCache {
321341
void clearCanonicalize(Uri url) {
322342
_canonicalizeCache.remove((url, forImport: false));
323343
_canonicalizeCache.remove((url, forImport: true));
324-
_relativeCanonicalizeCache.removeWhere((key, _) => key.$1 == url);
344+
_perImporterCanonicalizeCache.removeWhere(
345+
(key, _) => key.$2 == url || _nonCanonicalRelativeUrls[key] == url);
325346
}
326347

327348
/// Clears the cached parse tree for the stylesheet with the given

‎lib/src/util/map.dart‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
// MIT-style license that can be found in the LICENSE file or at
33
// https://opensource.org/licenses/MIT.
44

5+
import 'option.dart';
6+
57
extension MapExtensions<K, V> on Map<K, V> {
68
/// If [this] doesn't contain the given [key], sets that key to [value] and
79
/// returns it.
@@ -16,4 +18,8 @@ extension MapExtensions<K, V> on Map<K, V> {
1618
// TODO(nweiz): Remove this once dart-lang/collection#289 is released.
1719
/// Like [Map.entries], but returns each entry as a record.
1820
Iterable<(K, V)> get pairs => entries.map((e) => (e.key, e.value));
21+
22+
/// Returns an option that contains the value at [key] if one exists and null
23+
/// otherwise.
24+
Option<V> getOption(K key) => containsKey(key) ? (this[key]!,) : null;
1925
}

‎lib/src/util/option.dart‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// Copyright 2024 Google Inc. Use of this source code is governed by an
2+
// MIT-style license that can be found in the LICENSE file or at
3+
// https://opensource.org/licenses/MIT.
4+
5+
/// A type that represents either the presence of a value of type `T` or its
6+
/// absence.
7+
///
8+
/// When the option is present, this will be a single-element tuple that
9+
/// contains the value. If it's absent, it will be null. This allows callers to
10+
/// distinguish between a present null value and a value that's absent
11+
/// altogether.
12+
typedef Option<T> = (T,)?;

0 commit comments

Comments
 (0)