[dart:io] Fixes the FileIO performance regression

zichangg · commit-bot@chromium.org · commit ba25fd190ee3 · 2020-06-02T17:47:05.000Z
Previous CL (https://dart-review.googlesource.com/c/sdk/+/139881) causes performance regression. Improves: FileIO_readwrite_512kb: 196.5%, 224.1% FileIO_readwrite_verify_512kb: 38.28% 46.71% Change-Id: I5fb7752f14a2b39238e82bf45b059fe145b9087f Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/148563 Commit-Queue: Zichang Guo <zichangguo@google.com> Reviewed-by: Martin Kustermann <kustermann@google.com> Reviewed-by: Jonas Termansen <sortie@google.com>
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
@@ -4270,6 +4270,10 @@ bool Class::InjectCIDFields() const {
 #define ADD_SET_FIELD(clazz) {"cid" #clazz, kTypedData##clazz##Cid},
               CLASS_LIST_TYPED_DATA(ADD_SET_FIELD)
 #undef ADD_SET_FIELD
+#define ADD_SET_FIELD(clazz)                                                   \
+  {"cidExternal" #clazz, kExternalTypedData##clazz##Cid},
+                  CLASS_LIST_TYPED_DATA(ADD_SET_FIELD)
+#undef ADD_SET_FIELD
 #undef CLASS_LIST_WITH_NULL
   };
 
diff --git a/sdk/lib/_internal/js_dev_runtime/patch/io_patch.dart b/sdk/lib/_internal/js_dev_runtime/patch/io_patch.dart
@@ -218,7 +218,7 @@ class _RandomAccessFileOps {
 
 @patch
 bool _isDirectIOCapableTypedList(List<int> buffer) {
-  return buffer is Uint8List || buffer is Int8List;
+  return false;
 }
 
 @patch
diff --git a/sdk/lib/_internal/js_runtime/lib/io_patch.dart b/sdk/lib/_internal/js_runtime/lib/io_patch.dart
@@ -218,7 +218,7 @@ class _RandomAccessFileOps {
 
 @patch
 bool _isDirectIOCapableTypedList(List<int> buffer) {
-  return buffer is Uint8List || buffer is Int8List;
+  return false;
 }
 
 @patch
diff --git a/sdk/lib/_internal/vm/bin/common_patch.dart b/sdk/lib/_internal/vm/bin/common_patch.dart
@@ -54,7 +54,14 @@ import "dart:typed_data" show Uint8List;
 @patch
 bool _isDirectIOCapableTypedList(List<int> buffer) {
   int classID = ClassID.getID(buffer);
-  return classID == ClassID.cidUint8Array || classID == ClassID.cidInt8Array;
+  return classID == ClassID.cidUint8ArrayView ||
+      classID == ClassID.cidUint8Array ||
+      classID == ClassID.cidInt8ArrayView ||
+      classID == ClassID.cidInt8Array ||
+      classID == ClassID.cidExternalUInt8Array ||
+      classID == ClassID.cidExternalInt8Array ||
+      classID == ClassID.cidUint8ClampedArray ||
+      classID == ClassID.cidExternalUint8ClampedArray;
 }
 
 @patch
diff --git a/sdk/lib/_internal/vm/lib/class_id_fasta.dart b/sdk/lib/_internal/vm/lib/class_id_fasta.dart
@@ -29,5 +29,15 @@ class ClassID {
   @pragma("vm:entry-point")
   static final int cidUint8Array = 0;
   @pragma("vm:entry-point")
+  static final int cidInt8ArrayView = 0;
+  @pragma("vm:entry-point")
   static final int cidInt8Array = 0;
+  @pragma("vm:entry-point")
+  static final int cidExternalUInt8Array = 0;
+  @pragma("vm:entry-point")
+  static final int cidExternalInt8Array = 0;
+  @pragma("vm:entry-point")
+  static final int cidUint8ClampedArray = 0;
+  @pragma("vm:entry-point")
+  static final int cidExternalUint8ClampedArray = 0;
 }
diff --git a/sdk/lib/io/common.dart b/sdk/lib/io/common.dart
@@ -108,7 +108,7 @@ _BufferAndStart _ensureFastAndSerializableByteData(
   return new _BufferAndStart(newBuffer, 0);
 }
 
-// VM will use ClassID to check whether buffer is Uint8List or Int8List.
+// The VM will use ClassID to check whether buffer is Uint8List or Int8List.
 external bool _isDirectIOCapableTypedList(List<int> buffer);
 
 class _IOCrypto {
diff --git a/sdk_nnbd/lib/_internal/js_dev_runtime/patch/io_patch.dart b/sdk_nnbd/lib/_internal/js_dev_runtime/patch/io_patch.dart
@@ -216,7 +216,7 @@ class _RandomAccessFileOps {
 
 @patch
 bool _isDirectIOCapableTypedList(List<int> buffer) {
-  return buffer is Uint8List || buffer is Int8List;
+  return false;
 }
 
 @patch
diff --git a/sdk_nnbd/lib/_internal/js_runtime/lib/io_patch.dart b/sdk_nnbd/lib/_internal/js_runtime/lib/io_patch.dart
@@ -216,7 +216,7 @@ class _RandomAccessFileOps {
 
 @patch
 bool _isDirectIOCapableTypedList(List<int> buffer) {
-  return buffer is Uint8List || buffer is Int8List;
+  return false;
 }
 
 @patch
diff --git a/sdk_nnbd/lib/_internal/vm/bin/common_patch.dart b/sdk_nnbd/lib/_internal/vm/bin/common_patch.dart
@@ -52,7 +52,14 @@ import "dart:typed_data" show Uint8List;
 @patch
 bool _isDirectIOCapableTypedList(List<int> buffer) {
   int classID = ClassID.getID(buffer);
-  return classID == ClassID.cidUint8Array || classID == ClassID.cidInt8Array;
+  return classID == ClassID.cidUint8ArrayView ||
+      classID == ClassID.cidUint8Array ||
+      classID == ClassID.cidInt8ArrayView ||
+      classID == ClassID.cidInt8Array ||
+      classID == ClassID.cidExternalUint8Array ||
+      classID == ClassID.cidExternalInt8Array ||
+      classID == ClassID.cidUint8ClampedArray ||
+      classID == ClassID.cidExternalUint8ClampedArray;
 }
 
 @patch
diff --git a/sdk_nnbd/lib/_internal/vm/lib/class_id_fasta.dart b/sdk_nnbd/lib/_internal/vm/lib/class_id_fasta.dart
@@ -27,5 +27,15 @@ class ClassID {
   @pragma("vm:entry-point")
   static final int cidUint8Array = 0;
   @pragma("vm:entry-point")
+  static final int cidInt8ArrayView = 0;
+  @pragma("vm:entry-point")
   static final int cidInt8Array = 0;
+  @pragma("vm:entry-point")
+  static final int cidExternalUint8Array = 0;
+  @pragma("vm:entry-point")
+  static final int cidExternalInt8Array = 0;
+  @pragma("vm:entry-point")
+  static final int cidUint8ClampedArray = 0;
+  @pragma("vm:entry-point")
+  static final int cidExternalUint8ClampedArray = 0;
 }
diff --git a/sdk_nnbd/lib/io/common.dart b/sdk_nnbd/lib/io/common.dart
@@ -106,7 +106,7 @@ _BufferAndStart _ensureFastAndSerializableByteData(
   return new _BufferAndStart(newBuffer, 0);
 }
 
-// VM will use ClassID to check whether buffer is Uint8List or Int8List.
+// The VM will use ClassID to check whether buffer is Uint8List or Int8List.
 external bool _isDirectIOCapableTypedList(List<int> buffer);
 
 class _IOCrypto {
diff --git a/tests/standalone/io/file_write_as_test.dart b/tests/standalone/io/file_write_as_test.dart
@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'dart:async';
+import 'dart:collection';
 import 'dart:io';
 import 'dart:typed_data';
 
@@ -90,8 +91,40 @@ Future testWriteAsString(dir) {
 
 testWriteAsSubtypeSync(dir) {
   var f = new File('${dir.path}${Platform.pathSeparator}bytes_sync.txt');
-  f.writeAsBytesSync(UnmodifiableUint8ListView(Uint8List(10)));
-  Expect.equals(10, f.readAsBytesSync().length);
+  var input = Uint8List(5);
+  input[0] = 1;
+  input[1] = 2;
+  f.writeAsBytesSync(UnmodifiableUint8ListView(input));
+  var bytes = f.readAsBytesSync();
+  Expect.listEquals(input, bytes);
+}
+
+class MyUint8List extends ListBase<int> implements List<int> {
+  Uint8List _source;
+  MyUint8List(this._source);
+
+  // TypedData.
+  ByteBuffer get buffer => _source.buffer;
+  int get lengthInBytes => _source.lengthInBytes;
+  int get offsetInBytes => _source.offsetInBytes;
+
+  /// The methods that ListBase needs:
+  int operator [](int index) => _source[index];
+  operator []=(int index, int value) => _source[index] = value;
+  int get length => _source.length;
+  set length(_) => UnsupportedError("fixed length");
+  int get elementSizeInBytes => _source.elementSizeInBytes;
+}
+
+void testCustomizedSubtypeSync(Directory dir) {
+  var f = new File('${dir.path}${Platform.pathSeparator}bytes_sync.txt');
+  var input = Uint8List(5);
+  input[0] = 1;
+  input[1] = 2;
+  MyUint8List list = MyUint8List(input);
+  f.writeAsBytesSync(list);
+  var bytes = f.readAsBytesSync();
+  Expect.listEquals(input, bytes);
 }
 
 main() {
@@ -101,6 +134,7 @@ main() {
   testWriteAsStringSync(tempDir);
   testWriteWithLargeList(tempDir);
   testWriteAsSubtypeSync(tempDir);
+  testCustomizedSubtypeSync(tempDir);
   testWriteAsBytes(tempDir).then((_) {
     return testWriteAsString(tempDir);
   }).then((_) {
diff --git a/tests/standalone_2/io/file_write_as_test.dart b/tests/standalone_2/io/file_write_as_test.dart
@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'dart:async';
+import 'dart:collection';
 import 'dart:io';
 import 'dart:typed_data';
 
@@ -90,8 +91,40 @@ Future testWriteAsString(dir) {
 
 testWriteAsSubtypeSync(dir) {
   var f = new File('${dir.path}${Platform.pathSeparator}bytes_sync.txt');
-  f.writeAsBytesSync(UnmodifiableUint8ListView(Uint8List(10)));
-  Expect.equals(10, f.readAsBytesSync().length);
+  var input = Uint8List(5);
+  input[0] = 1;
+  input[1] = 2;
+  f.writeAsBytesSync(UnmodifiableUint8ListView(input));
+  var bytes = f.readAsBytesSync();
+  Expect.listEquals(input, bytes);
+}
+
+class MyUint8List extends ListBase<int> implements List<int> {
+  Uint8List _source;
+  MyUint8List(this._source);
+
+  // TypedData.
+  ByteBuffer get buffer => _source.buffer;
+  int get lengthInBytes => _source.lengthInBytes;
+  int get offsetInBytes => _source.offsetInBytes;
+
+  /// The methods that ListBase needs:
+  int operator [](int index) => _source[index];
+  operator []=(int index, int value) => _source[index] = value;
+  int get length => _source.length;
+  set length(_) => UnsupportedError("fixed length");
+  int get elementSizeInBytes => _source.elementSizeInBytes;
+}
+
+void testCustomizedSubtypeSync(Directory dir) {
+  var f = new File('${dir.path}${Platform.pathSeparator}bytes_sync.txt');
+  var input = Uint8List(5);
+  input[0] = 1;
+  input[1] = 2;
+  MyUint8List list = MyUint8List(input);
+  f.writeAsBytesSync(list);
+  var bytes = f.readAsBytesSync();
+  Expect.listEquals(input, bytes);
 }
 
 main() {
@@ -101,6 +134,7 @@ main() {
   testWriteAsStringSync(tempDir);
   testWriteWithLargeList(tempDir);
   testWriteAsSubtypeSync(tempDir);
+  testCustomizedSubtypeSync(tempDir);
   testWriteAsBytes(tempDir).then((_) {
     return testWriteAsString(tempDir);
   }).then((_) {

Original file line number	Diff line number	Diff line change
`@@ -218,7 +218,7 @@ class _RandomAccessFileOps {`
`218`	`218`
`219`	`219`	`@patch`
`220`	`220`	`bool _isDirectIOCapableTypedList(List<int> buffer) {`
`221`		`- return buffer is Uint8List \|\| buffer is Int8List;`
	`221`	`+ return false;`
`222`	`222`	`}`
`223`	`223`
`224`	`224`	`@patch`
Original file line number	Diff line number	Diff line change
`@@ -108,7 +108,7 @@ _BufferAndStart _ensureFastAndSerializableByteData(`
`108`	`108`	`return new _BufferAndStart(newBuffer, 0);`
`109`	`109`	`}`
`110`	`110`
`111`		`-// VM will use ClassID to check whether buffer is Uint8List or Int8List.`
	`111`	`+// The VM will use ClassID to check whether buffer is Uint8List or Int8List.`
`112`	`112`	`external bool _isDirectIOCapableTypedList(List<int> buffer);`
`113`	`113`
`114`	`114`	`class _IOCrypto {`
Original file line number	Diff line number	Diff line change
`@@ -216,7 +216,7 @@ class _RandomAccessFileOps {`
`216`	`216`
`217`	`217`	`@patch`
`218`	`218`	`bool _isDirectIOCapableTypedList(List<int> buffer) {`
`219`		`- return buffer is Uint8List \|\| buffer is Int8List;`
	`219`	`+ return false;`
`220`	`220`	`}`
`221`	`221`
`222`	`222`	`@patch`