[dart2js] Small improvements in emitting JS.

- Remove unused Map in source map emitter logic.
- Buffer output writing for both JS and source maps. Previously we were only buffering the source map writes.

During local compilations of a large program combined these amount to a >100MB improvement in memory usage.

Change-Id: I633c2f81aa28744e30c6a706bb3927423b38a6e0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/331361
Reviewed-by: Mayank Patke <fishythefish@google.com>
Commit-Queue: Nate Biggs <natebiggs@google.com>

pkg/compiler/lib/src/io/source_map_builder.dart

@@ -63,22 +63,12 @@ class SourceMapBuilder {
 
   void build() {
     LineColumnMap<SourceMapEntry> lineColumnMap = LineColumnMap();
-    Map<Uri, LineColumnMap<SourceMapEntry>> sourceLocationMap = {};
     entries.forEach((SourceMapEntry sourceMapEntry) {
       Location kernelLocation =
           locationProvider.getLocation(sourceMapEntry.targetOffset);
       int line = kernelLocation.line - 1;
       int column = kernelLocation.column - 1;
       lineColumnMap.add(line, column, sourceMapEntry);
-
-      SourceLocation location = sourceMapEntry.sourceLocation;
-      if (location.sourceUri != null) {
-        LineColumnMap<SourceMapEntry> sourceLineColumnMap =
-            sourceLocationMap.putIfAbsent(
-                location.sourceUri!, () => LineColumnMap<SourceMapEntry>());
-        sourceLineColumnMap.add(
-            location.line - 1, location.column - 1, sourceMapEntry);
-      }
     });
 
     _build(lineColumnMap);
@@ -282,7 +272,7 @@ class SourceMapBuilder {
           extension = 'js.map.${sourceLocations.name}';
         }
       }
-      final outputSink = BufferedStringOutputSink(compilerOutput
+      final outputSink = BufferedStringSinkWrapper(compilerOutput
           .createOutputSink(name, extension, api.OutputType.sourceMap));
       SourceMapBuilder sourceMapBuilder = SourceMapBuilder(
           sourceLocations.name,

pkg/compiler/lib/src/source_file_provider.dart

@@ -14,6 +14,7 @@ import '../compiler_api.dart' as api;
 import 'colors.dart' as colors;
 import 'common/metrics.dart';
 import 'io/source_file.dart';
+import 'util/output_util.dart';
 
 abstract class SourceFileByteReader {
   List<int> getBytes(String filename, {bool zeroTerminated = true});
@@ -396,36 +397,7 @@ class RandomAccessFileOutputProvider implements api.CompilerOutput {
 
     allOutputFiles.add(fe.relativizeUri(Uri.base, uri, Platform.isWindows));
 
-    int charactersWritten = 0;
-
-    void writeStringSync(String data) {
-      // Write the data in chunks of 8kb, otherwise we risk running OOM.
-      int chunkSize = 8 * 1024;
-
-      int offset = 0;
-      while (offset < data.length) {
-        String chunk;
-        int cut = offset + chunkSize;
-        if (cut < data.length) {
-          // Don't break the string in the middle of a code point encoded as two
-          // surrogate pairs since `writeStringSync` will encode the unpaired
-          // surrogates as U+FFFD REPLACEMENT CHARACTER.
-          int lastCodeUnit = data.codeUnitAt(cut - 1);
-          if (_isLeadSurrogate(lastCodeUnit)) {
-            cut -= 1;
-          }
-          chunk = data.substring(offset, cut);
-        } else {
-          chunk = offset == 0 ? data : data.substring(offset);
-        }
-        output.writeStringSync(chunk);
-        offset += chunk.length;
-      }
-      charactersWritten += data.length;
-    }
-
-    void onDone() {
-      output.closeSync();
+    void onClose(int charactersWritten) {
       totalCharactersWritten += charactersWritten;
       if (isPrimaryOutput) {
         totalCharactersWrittenPrimary += charactersWritten;
@@ -435,11 +407,10 @@ class RandomAccessFileOutputProvider implements api.CompilerOutput {
       }
     }
 
-    return _OutputSinkWrapper(writeStringSync, onDone);
+    return BufferedStringSinkWrapper(
+        FileStringOutputSink(output, onClose: onClose));
   }
 
-  static bool _isLeadSurrogate(int codeUnit) => (codeUnit & 0xFC00) == 0xD800;
-
   @override
   api.BinaryOutputSink createBinarySink(Uri uri) {
     uri = Uri.base.resolveUri(uri);
@@ -460,19 +431,11 @@ class RandomAccessFileOutputProvider implements api.CompilerOutput {
       throw StateError('unreachable');
     }
 
-    int bytesWritten = 0;
-
-    void writeBytesSync(List<int> data, [int start = 0, int? end]) {
-      output.writeFromSync(data, start, end);
-      bytesWritten += (end ?? data.length) - start;
-    }
-
-    void onDone() {
-      output.closeSync();
+    void onClose(int bytesWritten) {
       totalDataWritten += bytesWritten;
     }
 
-    return _BinaryOutputSinkWrapper(writeBytesSync, onDone);
+    return FileBinaryOutputSink(output, onClose: onClose);
   }
 }
 
@@ -493,33 +456,6 @@ class RandomAccessBinaryOutputSink implements api.BinaryOutputSink {
   }
 }
 
-class _OutputSinkWrapper extends api.OutputSink {
-  void Function(String) onAdd;
-  void Function() onClose;
-
-  _OutputSinkWrapper(this.onAdd, this.onClose);
-
-  @override
-  void add(String data) => onAdd(data);
-
-  @override
-  void close() => onClose();
-}
-
-class _BinaryOutputSinkWrapper extends api.BinaryOutputSink {
-  void Function(List<int>, [int, int?]) onWrite;
-  void Function() onClose;
-
-  _BinaryOutputSinkWrapper(this.onWrite, this.onClose);
-
-  @override
-  void add(List<int> data, [int start = 0, int? end]) =>
-      onWrite(data, start, end);
-
-  @override
-  void close() => onClose();
-}
-
 /// Adapter to integrate dart2js in bazel.
 ///
 /// To handle bazel's special layout:

pkg/compiler/lib/src/util/output_util.dart

@@ -1,59 +1,161 @@
+import 'dart:io';
+
 import '../../compiler_api.dart' as api;
 
+/// Implementation of [api.BinaryOutputSink] that writes to a provided file.
+class FileBinaryOutputSink extends api.BinaryOutputSink {
+  final RandomAccessFile _fileOut;
+  void Function(int bytesWritten)? onClose;
+  int _bytesWritten = 0;
+
+  FileBinaryOutputSink(this._fileOut, {this.onClose});
+
+  @override
+  void add(List<int> data, [int start = 0, int? end]) {
+    _fileOut.writeFromSync(data, start, end);
+    _bytesWritten += (end ?? data.length) - start;
+  }
+
+  @override
+  void close() {
+    _fileOut.closeSync();
+    if (onClose != null) {
+      onClose!(_bytesWritten);
+    }
+  }
+}
+
+/// Implementation of [api.OutputSink] that writes to a provided file.
+class FileStringOutputSink implements api.OutputSink {
+  final RandomAccessFile _fileOut;
+  void Function(int charactersWritten)? onClose;
+  int _charactersWritten = 0;
+
+  FileStringOutputSink(this._fileOut, {this.onClose});
+
+  @override
+  void add(String text) {
+    _fileOut.writeStringSync(text);
+    _charactersWritten += text.length;
+  }
+
+  @override
+  void close() {
+    _fileOut.closeSync();
+    if (onClose != null) {
+      onClose!(_charactersWritten);
+    }
+  }
+}
+
 /// [StringSink] that buffers data written to the underlying [api.OutputSink].
 ///
-/// Useful for string that are built slowly over many writes, avoids keeping the
-/// entire string in memory. Each write operation checks if the length of the
-/// buffer is above the [maxLength] threshold but a single write can push the
-/// buffer above that threshold meaning more than [maxLength] bytes may be
-/// forwarded at once. Avoid large writes to ensure the buffer is effective.
-class BufferedStringOutputSink implements StringSink {
-  StringBuffer buffer = StringBuffer();
-  final api.OutputSink outputSink;
-  final int maxLength;
-  static const int _defaultMaxLength = 1024;
+/// Useful for strings that are built slowly over many writes, avoids keeping
+/// the entire string in memory. Each write operation checks if the internal
+/// buffer is longer than a maximum length and writes the contents to the
+/// underlying [api.OutputSink] if so. Chunks large writes to prevent OOM
+/// issues.
+///
+/// When wrapping an [api.OutputSink], will check if the provided sink is itself
+/// a [BufferedStringSinkWrapper]. If it is the same object will be returned,
+/// otherwise returns a new [BufferedStringSinkWrapper] wrapping the provided
+/// sink.
+///
+/// Avoid large writes to a [BufferedStringSinkWrapper] to ensure the buffer is
+/// effective.
+class BufferedStringSinkWrapper implements api.OutputSink, StringSink {
+  static const _maxBufferSize = 1024;
 
-  BufferedStringOutputSink(this.outputSink,
-      {this.maxLength = _defaultMaxLength});
+  /// This should be at most 8kb, otherwise we risk running OOM. If
+  /// [_maxBufferSize] is less than [_maxChunkSize] then this chunking will only
+  /// be used if a single write exceeds this size.
+  static const _maxChunkSize = 8 * 1024;
 
+  final _buffer = StringBuffer();
+  final api.OutputSink _outputSink;
+
+  BufferedStringSinkWrapper._(this._outputSink);
+
+  factory BufferedStringSinkWrapper(api.OutputSink _outputSink) {
+    return _outputSink is BufferedStringSinkWrapper
+        ? _outputSink
+        : BufferedStringSinkWrapper._(_outputSink);
+  }
+
+  static bool _isLeadSurrogate(int codeUnit) => (codeUnit & 0xFC00) == 0xD800;
+
+  /// Write the data in chunks if it is too large for a single write.
+  void _flushChunked(String data) {
+    int offset = 0;
+    while (offset < data.length) {
+      String chunk;
+      int cut = offset + _maxChunkSize;
+      if (cut < data.length) {
+        // Don't break the string in the middle of a code point encoded as two
+        // surrogate pairs since `writeStringSync` will encode the unpaired
+        // surrogates as U+FFFD REPLACEMENT CHARACTER.
+        int lastCodeUnit = data.codeUnitAt(cut - 1);
+        if (_isLeadSurrogate(lastCodeUnit)) {
+          cut -= 1;
+        }
+        chunk = data.substring(offset, cut);
+      } else {
+        chunk = offset == 0 ? data : data.substring(offset);
+      }
+      _outputSink.add(chunk);
+      offset += chunk.length;
+    }
+  }
+
+  void _flush() {
+    final data = _buffer.toString();
+    if (data.length > _maxChunkSize) {
+      _flushChunked(data);
+    } else {
+      _outputSink.add(data);
+    }
+    _buffer.clear();
+  }
+
+  void _flushIfNecessary() {
+    if (_buffer.length >= _maxBufferSize) {
+      _flush();
+    }
+  }
+
+  @override
+  void add(String data) {
+    _buffer.write(data);
+    _flushIfNecessary();
+  }
+
+  @override
   void close() {
-    outputSink.add(buffer.toString());
-    outputSink.close();
+    _flush();
+    _outputSink.close();
   }
 
   @override
   void write(Object? object) {
-    buffer.write(object);
-    if (buffer.length > maxLength) {
-      outputSink.add(buffer.toString());
-      buffer.clear();
-    }
+    _buffer.write(object);
+    _flushIfNecessary();
   }
 
   @override
   void writeAll(Iterable objects, [String separator = ""]) {
-    buffer.writeAll(objects, separator);
-    if (buffer.length > maxLength) {
-      outputSink.add(buffer.toString());
-      buffer.clear();
-    }
+    _buffer.writeAll(objects, separator);
+    _flushIfNecessary();
   }
 
   @override
   void writeCharCode(int charCode) {
-    buffer.writeCharCode(charCode);
-    if (buffer.length > maxLength) {
-      outputSink.add(buffer.toString());
-      buffer.clear();
-    }
+    _buffer.writeCharCode(charCode);
+    _flushIfNecessary();
   }
 
   @override
   void writeln([Object? object = ""]) {
-    buffer.writeln(object);
-    if (buffer.length > maxLength) {
-      outputSink.add(buffer.toString());
-      buffer.clear();
-    }
+    _buffer.writeln(object);
+    _flushIfNecessary();
   }
 }