Skip to content

Add back write caching. #4001

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions _test_common/lib/runner_asset_writer_spy.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,4 @@ class RunnerAssetWriterSpy extends AssetWriterSpy implements RunnerAssetWriter {

@override
Future<void> deleteDirectory(AssetId id) => _delegate.deleteDirectory(id);

@override
Future<void> completeBuild() async {}
}
6 changes: 5 additions & 1 deletion _test_common/lib/test_environment.dart
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,12 @@ class TestBuildEnvironment implements BuildEnvironment {
BuildEnvironment copyWith({
void Function(LogRecord)? onLogOverride,
RunnerAssetWriter? writer,
AssetReader? reader,
}) => TestBuildEnvironment(
readerWriter: (writer as TestReaderWriter?) ?? _readerWriter,
readerWriter:
(writer as TestReaderWriter?) ??
(reader as TestReaderWriter?) ??
_readerWriter,
throwOnPrompt: throwOnPrompt,
);

Expand Down
153 changes: 146 additions & 7 deletions build/lib/src/state/filesystem_cache.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'dart:async';
import 'dart:convert';
import 'dart:typed_data';

import '../../build.dart';
import '../asset/id.dart';
import 'lru_cache.dart';

Expand All @@ -16,6 +17,9 @@ abstract interface class FilesystemCache {
/// Clears all [ids] from all caches.
void invalidate(Iterable<AssetId> ids);

/// Flushes pending writes and deletes.
void flush();

/// Whether [id] exists.
///
/// Returns a cached result if available, or caches and returns `ifAbsent()`.
Expand All @@ -26,6 +30,17 @@ abstract interface class FilesystemCache {
/// Returns a cached result if available, or caches and returns `ifAbsent()`.
Uint8List readAsBytes(AssetId id, {required Uint8List Function() ifAbsent});

/// Writes [contents] to [id].
///
/// [writer] is a function that does the actual write. If this cache does
/// write caching, it is not called until [flush], and might not be called at
/// all if another write to the same asset happens first.
void writeAsBytes(
AssetId id,
List<int> contents, {
required void Function() writer,
});

/// Reads [id] as a `String`.
///
/// Returns a cached result if available, or caches and returns `ifAbsent()`.
Expand All @@ -34,6 +49,25 @@ abstract interface class FilesystemCache {
Encoding encoding = utf8,
required Uint8List Function() ifAbsent,
});

/// Writes [contents] to [id].
///
/// [writer] is a function that does the actual write. If this cache does
/// write caching, it is not called until [flush], and might not be called at
/// all if another write to the same asset happens first.
void writeAsString(
AssetId id,
String contents, {
Encoding encoding = utf8,
required void Function() writer,
});

/// Deletes [id].
///
/// [deleter] is a function that does the actual delete. If this cache does
/// write caching, it is not called until [flush], and might not be called at
/// all if another write to the same asset happens first.
void delete(AssetId id, {required void Function() deleter});
}

/// [FilesystemCache] that always reads from the underlying source.
Expand All @@ -43,19 +77,40 @@ class PassthroughFilesystemCache implements FilesystemCache {
@override
Future<void> invalidate(Iterable<AssetId> ids) async {}

@override
void flush() {}

@override
bool exists(AssetId id, {required bool Function() ifAbsent}) => ifAbsent();

@override
Uint8List readAsBytes(AssetId id, {required Uint8List Function() ifAbsent}) =>
ifAbsent();

@override
void writeAsBytes(
AssetId id,
List<int> contents, {
required void Function() writer,
}) => writer();

@override
String readAsString(
AssetId id, {
Encoding encoding = utf8,
required Uint8List Function() ifAbsent,
}) => encoding.decode(ifAbsent());

@override
void writeAsString(
AssetId id,
String contents, {
Encoding encoding = utf8,
required void Function() writer,
}) => writer();

@override
void delete(AssetId id, {required void Function() deleter}) => deleter();
}

/// [FilesystemCache] that stores data in memory.
Expand Down Expand Up @@ -83,8 +138,13 @@ class InMemoryFilesystemCache implements FilesystemCache {
(value) => value.length,
);

final _pendingWrites = <AssetId, _PendingWrite>{};

@override
Future<void> invalidate(Iterable<AssetId> ids) async {
if (_pendingWrites.isNotEmpty) {
throw StateError("Can't invalidate while there are pending writes.");
}
for (var id in ids) {
_existsCache.remove(id);
_bytesContentCache.remove(id);
Expand All @@ -93,11 +153,30 @@ class InMemoryFilesystemCache implements FilesystemCache {
}

@override
bool exists(AssetId id, {required bool Function() ifAbsent}) =>
_existsCache.putIfAbsent(id, ifAbsent);
void flush() {
for (final write in _pendingWrites.values) {
write.writer();
}
_pendingWrites.clear();
}

@override
bool exists(AssetId id, {required bool Function() ifAbsent}) {
final maybePendingWrite = _pendingWrites[id];
if (maybePendingWrite != null) {
return !maybePendingWrite.isDelete;
}
return _existsCache.putIfAbsent(id, ifAbsent);
}

@override
Uint8List readAsBytes(AssetId id, {required Uint8List Function() ifAbsent}) {
final maybePendingWrite = _pendingWrites[id];
if (maybePendingWrite != null) {
// Throws if it's a delete; callers should check [exists] before reading.
return maybePendingWrite.bytes!;
}

final maybeResult = _bytesContentCache[id];
if (maybeResult != null) return maybeResult;

Expand All @@ -106,27 +185,87 @@ class InMemoryFilesystemCache implements FilesystemCache {
return result;
}

@override
void writeAsBytes(
AssetId id,
List<int> contents, {
required void Function() writer,
}) {
_stringContentCache.remove(id);
_writeBytes(id, contents, writer: writer);
}

void _writeBytes(
AssetId id,
List<int> contents, {
required void Function() writer,
}) {
final uint8ListContents =
contents is Uint8List ? contents : Uint8List.fromList(contents);
_bytesContentCache[id] = uint8ListContents;
_existsCache[id] = true;
_pendingWrites[id] = _PendingWrite(
writer: writer,
bytes: uint8ListContents,
);
}

@override
String readAsString(
AssetId id, {
Encoding encoding = utf8,
required Uint8List Function() ifAbsent,
}) {
// Encodings other than utf8 do not use `_stringContentCache`. Read as
// bytes then convert, instead.
if (encoding != utf8) {
final bytes = readAsBytes(id, ifAbsent: ifAbsent);
return encoding.decode(bytes);
}

// Check `_stringContentCache` first to use it as a cache for conversion of
Copy link

Choose a reason for hiding this comment

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

I can't seem to put a comment at the right place, but line 223-235 could basically be replaced by a call like final bytes = readAsBytes(id, ifAbsent: ifAbsent); (as above).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it can ... thanks! Done.

// bytes from _pendingWrites.
final maybeResult = _stringContentCache[id];
if (maybeResult != null) return maybeResult;

var bytes = _bytesContentCache[id];
if (bytes == null) {
bytes = ifAbsent();
_bytesContentCache[id] = bytes;
}
final bytes = readAsBytes(id, ifAbsent: ifAbsent);
final result = utf8.decode(bytes);
_stringContentCache[id] = result;
return result;
}

@override
void writeAsString(
AssetId id,
String contents, {
Encoding encoding = utf8,
required void Function() writer,
}) {
// Encodings other than utf8 do not use `_stringContentCache`.
if (encoding == utf8) {
_stringContentCache[id] = contents;
} else {
_stringContentCache.remove(id);
}
Copy link

Choose a reason for hiding this comment

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

and this stuff could (almost) be replaced by a call to writeAsBytes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks; extracted common _writeBytes.

final bytes = encoding.encode(contents);
_writeBytes(id, bytes, writer: writer);
}

@override
void delete(AssetId id, {required void Function() deleter}) {
_stringContentCache.remove(id);
_bytesContentCache.remove(id);
_existsCache[id] = false;
_pendingWrites[id] = _PendingWrite(writer: deleter);
}
}

/// The data that will be written on flush; used for reads before flush.
class _PendingWrite {
final void Function() writer;
final Uint8List? bytes;

_PendingWrite({required this.writer, this.bytes});

bool get isDelete => bytes == null;
}
Loading
Loading