[vm/io] Shrink buffers using realloc() if actual used data is small

Issue b/178993708

TEST=standalone{,_2}/io/large_file_read_small_file_test

Change-Id: I3c6f09f4f79f6dd1150db2918cefed3587e34012
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/182268
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>

runtime/bin/dartutils.cc

@@ -946,6 +946,32 @@ Dart_CObject* CObject::NewIOBuffer(int64_t length) {
                                IOBuffer::Finalizer);
 }
 
+void CObject::ShrinkIOBuffer(Dart_CObject* cobject, int64_t new_length) {
+  if (cobject == nullptr) return;
+  ASSERT(cobject->type == Dart_CObject_kExternalTypedData);
+
+  const auto old_data = cobject->value.as_external_typed_data.data;
+  const auto old_length = cobject->value.as_external_typed_data.length;
+
+  // We only shrink IOBuffers, never grow them.
+  ASSERT(0 <= new_length && new_length <= old_length);
+
+  // We only reallocate if we think the freed space is worth reallocating.
+  // We consider it worthwhile when freed space is >=25% and we have at
+  // least 100 free bytes.
+  const auto free_memory = old_length - new_length;
+  if ((old_length >> 2) <= free_memory && 100 <= free_memory) {
+    const auto new_data = IOBuffer::Reallocate(old_data, new_length);
+    if (new_data != nullptr) {
+      cobject->value.as_external_typed_data.data = new_data;
+      cobject->value.as_external_typed_data.peer = new_data;
+    }
+  }
+
+  // The typed data object always has to have the shranken length.
+  cobject->value.as_external_typed_data.length = new_length;
+}
+
 void CObject::FreeIOBufferData(Dart_CObject* cobject) {
   ASSERT(cobject->type == Dart_CObject_kExternalTypedData);
   cobject->value.as_external_typed_data.callback(

runtime/bin/dartutils.h

@@ -353,6 +353,7 @@ class CObject {
                                              Dart_HandleFinalizer callback);
 
   static Dart_CObject* NewIOBuffer(int64_t length);
+  static void ShrinkIOBuffer(Dart_CObject* cobject, int64_t new_length);
   static void FreeIOBufferData(Dart_CObject* object);
 
   Dart_CObject* AsApiCObject() { return cobject_; }
@@ -568,9 +569,6 @@ class CObjectExternalUint8Array : public CObject {
   intptr_t Length() const {
     return cobject_->value.as_external_typed_data.length;
   }
-  void SetLength(intptr_t length) {
-    cobject_->value.as_external_typed_data.length = length;
-  }
   uint8_t* Data() const { return cobject_->value.as_external_typed_data.data; }
   void* Peer() const { return cobject_->value.as_external_typed_data.peer; }
   Dart_HandleFinalizer Callback() const {

runtime/bin/file.cc

@@ -1246,9 +1246,12 @@ CObject* File::ReadRequest(const CObjectArray& request) {
     CObject::FreeIOBufferData(io_buffer);
     return CObject::NewOSError();
   }
-  CObjectExternalUint8Array* external_array =
-      new CObjectExternalUint8Array(io_buffer);
-  external_array->SetLength(bytes_read);
+
+  // Possibly shrink the used malloc() storage if the actual number of bytes is
+  // significantly lower.
+  CObject::ShrinkIOBuffer(io_buffer, bytes_read);
+
+  auto external_array = new CObjectExternalUint8Array(io_buffer);
   CObjectArray* result = new CObjectArray(CObject::NewArray(2));
   result->SetAt(0, new CObjectIntptr(CObject::NewInt32(0)));
   result->SetAt(1, external_array);
@@ -1278,9 +1281,12 @@ CObject* File::ReadIntoRequest(const CObjectArray& request) {
     CObject::FreeIOBufferData(io_buffer);
     return CObject::NewOSError();
   }
-  CObjectExternalUint8Array* external_array =
-      new CObjectExternalUint8Array(io_buffer);
-  external_array->SetLength(bytes_read);
+
+  // Possibly shrink the used malloc() storage if the actual number of bytes is
+  // significantly lower.
+  CObject::ShrinkIOBuffer(io_buffer, bytes_read);
+
+  auto external_array = new CObjectExternalUint8Array(io_buffer);
   CObjectArray* result = new CObjectArray(CObject::NewArray(3));
   result->SetAt(0, new CObjectIntptr(CObject::NewInt32(0)));
   result->SetAt(1, new CObjectInt64(CObject::NewInt64(bytes_read)));

runtime/bin/io_buffer.cc

@@ -28,7 +28,23 @@ Dart_Handle IOBuffer::Allocate(intptr_t size, uint8_t** buffer) {
 }
 
 uint8_t* IOBuffer::Allocate(intptr_t size) {
-  return reinterpret_cast<uint8_t*>(calloc(size, sizeof(uint8_t)));
+  return static_cast<uint8_t*>(calloc(size, sizeof(uint8_t)));
+}
+
+uint8_t* IOBuffer::Reallocate(uint8_t* buffer, intptr_t new_size) {
+  if (new_size == 0) {
+    // The call to `realloc()` below has a corner case if the new size is 0:
+    // It can return `nullptr` in that case even though `malloc(0)` would
+    // return a unique non-`nullptr` value.  To avoid returning `nullptr` on
+    // successful realloc, we handle this case specially.
+    auto new_buffer = IOBuffer::Allocate(0);
+    if (new_buffer != nullptr) {
+      free(buffer);
+      return new_buffer;
+    }
+    return buffer;
+  }
+  return static_cast<uint8_t*>(realloc(buffer, new_size));
 }
 
 }  // namespace bin

runtime/bin/io_buffer.h

@@ -20,6 +20,9 @@ class IOBuffer {
   // Allocate IO buffer storage.
   static uint8_t* Allocate(intptr_t size);
 
+  // Reallocate IO buffer storage.
+  static uint8_t* Reallocate(uint8_t* buffer, intptr_t new_size);
+
   // Function for disposing of IO buffer storage. All backing storage
   // for IO buffers must be freed using this function.
   static void Free(void* buffer) { free(buffer); }

tests/standalone/io/large_file_read_small_file_test.dart

@@ -0,0 +1,54 @@
+// Copyright (c) 2021, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:io';
+
+import 'package:expect/expect.dart';
+import 'package:path/path.dart' as path;
+
+Future runWithTempDir(Future Function(String dir) callback) async {
+  final tempDir = Directory.systemTemp.createTempSync("large_file_read");
+  try {
+    await callback(tempDir.path);
+  } finally {
+    tempDir.delete(recursive: true);
+  }
+}
+
+main() async {
+  // MSAN's malloc implementation will not free memory when shrinking
+  // allocations via realloc (e.g. realloc(malloc(1 GB), new_size=10) will
+  // hold on to the 1 GB).
+  if (Platform.executable.contains('MSAN')) return;
+
+  await runWithTempDir((String dir) async {
+    final file = File(path.join(dir, 'hello_world.txt'));
+    await file.writeAsString('hello world');
+    final RandomAccessFile randomAccessFile = await file.open();
+
+    try {
+      final buffers = [];
+      for (int i = 0; i < 10 * 1000; ++i) {
+        // We issue a 10 MB read but get only a small typed data back. We hang on
+        // to those buffers. If the implementation actually malloc()ed 10 MB then
+        // we would hang on to 100 GB and this test would OOM.
+        // If the implementation instead correctly shrinks the buffer before
+        // giving it to Dart as external typed data, we only consume ~ 100 KB.
+        buffers.add(await randomAccessFile.read(10 * 1024 * 1024));
+        await randomAccessFile.setPosition(0);
+
+        // To avoid machines becoming unusable if the test fails, we'll fail
+        // explicitly if we hit 2 GB.
+        if (ProcessInfo.currentRss > 2 * 1024 * 1024 * 1024) {
+          throw 'The dart:io implementation is buggy and uses too much memory';
+        }
+      }
+      for (final buffer in buffers) {
+        Expect.equals('hello world'.length, buffer.length);
+      }
+    } finally {
+      randomAccessFile.close();
+    }
+  });
+}

tests/standalone_2/io/large_file_read_small_file_test.dart

@@ -0,0 +1,54 @@
+// Copyright (c) 2021, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:io';
+
+import 'package:expect/expect.dart';
+import 'package:path/path.dart' as path;
+
+Future runWithTempDir(Future Function(String dir) callback) async {
+  final tempDir = Directory.systemTemp.createTempSync("large_file_read");
+  try {
+    await callback(tempDir.path);
+  } finally {
+    tempDir.delete(recursive: true);
+  }
+}
+
+main() async {
+  // MSAN's malloc implementation will not free memory when shrinking
+  // allocations via realloc (e.g. realloc(malloc(1 GB), new_size=10) will
+  // hold on to the 1 GB).
+  if (Platform.executable.contains('MSAN')) return;
+
+  await runWithTempDir((String dir) async {
+    final file = File(path.join(dir, 'hello_world.txt'));
+    await file.writeAsString('hello world');
+    final RandomAccessFile randomAccessFile = await file.open();
+
+    try {
+      final buffers = [];
+      for (int i = 0; i < 10 * 1000; ++i) {
+        // We issue a 10 MB read but get only a small typed data back. We hang on
+        // to those buffers. If the implementation actually malloc()ed 10 MB then
+        // we would hang on to 100 GB and this test would OOM.
+        // If the implementation instead correctly shrinks the buffer before
+        // giving it to Dart as external typed data, we only consume ~ 100 KB.
+        buffers.add(await randomAccessFile.read(10 * 1024 * 1024));
+        await randomAccessFile.setPosition(0);
+
+        // To avoid machines becoming unusable if the test fails, we'll fail
+        // explicitly if we hit 2 GB.
+        if (ProcessInfo.currentRss > 2 * 1024 * 1024 * 1024) {
+          throw 'The dart:io implementation is buggy and uses too much memory';
+        }
+      }
+      for (final buffer in buffers) {
+        Expect.equals('hello world'.length, buffer.length);
+      }
+    } finally {
+      randomAccessFile.close();
+    }
+  });
+}