[VM] Inline ClassID.getID() eagerly, extend pattern matching logic to recognize it, use it to special case ascii decoding
This change extends/fixes the exiting "pattern recognition" which tries
to recognize the pattern
v2 <- LoadClassIdInstr(v1)
BranchIf v2 == IntegerConstant(cid)
Furthermore we start inlining the recognized `ClassID.getID` method very
early in the pipeline. This allows the VM to recognize the above
pattern and insert redefinitions before the actual inlining pass.
Furthermore we special-case two very hot methods in utf8 decoding by
manually having two loops, one of which is guarded by a class-id check
against the _Uint8ArrayView class, which is most common. (In the future
we would like to unify the typed data layouts so we no longer need to
use `ClassId.getID`, thereby also allowing non core library code to use
this).
This improves dart-aot by
* 31%+ for a protobuf decoding benchmark we care about
Issue https://github.com/dart-lang/sdk/issues/31954
Change-Id: Ia567b92b7e76ff28eda1726deaafda32732ed8f5
Reviewed-on: https://dart-review.googlesource.com/c/85281
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Jenny Messerly <jmesserly@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
diff --git a/pkg/dev_compiler/tool/input_sdk/patch/convert_patch.dart b/pkg/dev_compiler/tool/input_sdk/patch/convert_patch.dart
index cfaa3fc..787c677 100644
--- a/pkg/dev_compiler/tool/input_sdk/patch/convert_patch.dart
+++ b/pkg/dev_compiler/tool/input_sdk/patch/convert_patch.dart
@@ -495,3 +495,13 @@
return null;
}();
}
+
+@patch
+int _scanOneByteCharacters(List<int> units, int from, int endIndex) {
+ final to = endIndex;
+ for (var i = from; i < to; i++) {
+ final unit = units[i];
+ if ((unit & _ONE_BYTE_LIMIT) != unit) return i - from;
+ }
+ return to - from;
+}
diff --git a/runtime/bin/BUILD.gn b/runtime/bin/BUILD.gn
index d3bc4df..a5c2317 100644
--- a/runtime/bin/BUILD.gn
+++ b/runtime/bin/BUILD.gn
@@ -342,9 +342,7 @@
"..:dart_config",
"..:dart_os_config",
] + extra_configs
- public_configs = [
- "..:dart_public_config",
- ]
+ public_configs = [ "..:dart_public_config" ]
if (is_fuchsia) {
configs -= [ "//build/config:symbol_visibility_hidden" ]
}
diff --git a/runtime/bin/process_fuchsia.cc b/runtime/bin/process_fuchsia.cc
index f24027e2..1608bc9 100644
--- a/runtime/bin/process_fuchsia.cc
+++ b/runtime/bin/process_fuchsia.cc
@@ -615,9 +615,9 @@
char err_msg[FDIO_SPAWN_ERR_MSG_MAX_LENGTH];
uint32_t flags = FDIO_SPAWN_CLONE_JOB | FDIO_SPAWN_CLONE_LDSVC |
FDIO_SPAWN_CLONE_NAMESPACE;
- status = fdio_spawn_vmo(ZX_HANDLE_INVALID, flags, vmo, program_arguments_,
- program_environment_, 4, actions, &process,
- err_msg);
+ status =
+ fdio_spawn_vmo(ZX_HANDLE_INVALID, flags, vmo, program_arguments_,
+ program_environment_, 4, actions, &process, err_msg);
if (status != ZX_OK) {
LOG_ERR("ProcessStarter: Start() fdio_spawn_vmo failed\n");
diff --git a/runtime/lib/class_id_fasta.dart b/runtime/lib/class_id_fasta.dart
index 0a89db4..c2f4ae0 100644
--- a/runtime/lib/class_id_fasta.dart
+++ b/runtime/lib/class_id_fasta.dart
@@ -14,4 +14,5 @@
static final int cidImmutableArray = 0;
static final int cidOneByteString = 0;
static final int cidTwoByteString = 0;
+ static final int cidUint8ArrayView = 0;
}
diff --git a/runtime/lib/convert_patch.dart b/runtime/lib/convert_patch.dart
index 0802cbb..3d99c0d 100644
--- a/runtime/lib/convert_patch.dart
+++ b/runtime/lib/convert_patch.dart
@@ -7,7 +7,7 @@
/// used by patches of that library. We plan to change this when we have a
/// shared front end and simply use parts.
-import "dart:_internal" show POWERS_OF_TEN, patch;
+import "dart:_internal" show POWERS_OF_TEN, patch, ClassID;
import "dart:typed_data" show Uint8List, Uint16List;
@@ -1816,3 +1816,27 @@
_sink.close();
}
}
+
+@patch
+int _scanOneByteCharacters(List<int> units, int from, int endIndex) {
+ final to = endIndex;
+
+ // Special case for _Uint8ArrayView.
+ final cid = ClassID.getID(units);
+ if (identical(cid, ClassID.cidUint8ArrayView)) {
+ if (from >= 0 && to >= 0 && to <= units.length) {
+ for (int i = from; i < to; i++) {
+ final unit = units[i];
+ if ((unit & _ONE_BYTE_LIMIT) != unit) return i - from;
+ }
+ return to - from;
+ }
+ }
+
+ // Fall through to normal case.
+ for (var i = from; i < to; i++) {
+ final unit = units[i];
+ if ((unit & _ONE_BYTE_LIMIT) != unit) return i - from;
+ }
+ return to - from;
+}
diff --git a/runtime/lib/string_patch.dart b/runtime/lib/string_patch.dart
index 103959b..e90667a 100644
--- a/runtime/lib/string_patch.dart
+++ b/runtime/lib/string_patch.dart
@@ -222,6 +222,19 @@
static String _createOneByteString(List<int> charCodes, int start, int len) {
// It's always faster to do this in Dart than to call into the runtime.
var s = _OneByteString._allocate(len);
+
+ // Special case for _Uint8ArrayView.
+ final cid = ClassID.getID(charCodes);
+ if (identical(cid, ClassID.cidUint8ArrayView)) {
+ if (start >= 0 && len >= 0) {
+ for (int i = 0; i < len; i++) {
+ s._setAt(i, charCodes[start + i]);
+ }
+ return s;
+ }
+ }
+
+ // Fall through to normal case.
for (int i = 0; i < len; i++) {
s._setAt(i, charCodes[start + i]);
}
diff --git a/runtime/vm/compiler/backend/inliner.cc b/runtime/vm/compiler/backend/inliner.cc
index bb5778a..61a6a51 100644
--- a/runtime/vm/compiler/backend/inliner.cc
+++ b/runtime/vm/compiler/backend/inliner.cc
@@ -2692,6 +2692,22 @@
return true;
}
+static bool InlineLoadClassId(FlowGraph* flow_graph,
+ Instruction* call,
+ GraphEntryInstr* graph_entry,
+ FunctionEntryInstr** entry,
+ Instruction** last) {
+ *entry =
+ new (Z) FunctionEntryInstr(graph_entry, flow_graph->allocate_block_id(),
+ call->GetBlock()->try_index(), DeoptId::kNone);
+ (*entry)->InheritDeoptTarget(Z, call);
+ auto load_cid = new (Z)
+ LoadClassIdInstr(call->PushArgumentAt(0)->value()->CopyWithType(Z));
+ flow_graph->InsertBefore(call, load_cid, nullptr, FlowGraph::kValue);
+ *last = load_cid;
+ return true;
+}
+
// Adds an explicit bounds check for a typed getter/setter.
static void PrepareInlineTypedArrayBoundsCheck(FlowGraph* flow_graph,
Instruction* call,
@@ -3605,6 +3621,8 @@
}
return InlineGetIndexed(flow_graph, kind, call, receiver, graph_entry,
entry, last, can_speculate);
+ case MethodRecognizer::kClassIDgetID:
+ return InlineLoadClassId(flow_graph, call, graph_entry, entry, last);
default:
break;
}
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
index 80b2c50..c81689d 100644
--- a/runtime/vm/object.cc
+++ b/runtime/vm/object.cc
@@ -3707,6 +3707,18 @@
CLASS_LIST_WITH_NULL(ADD_SET_FIELD)
#undef ADD_SET_FIELD
+
+#define ADD_SET_FIELD(clazz) \
+ field_name = Symbols::New(thread, "cid" #clazz "View"); \
+ field = Field::New(field_name, true, false, true, false, *this, \
+ Type::Handle(Type::IntType()), TokenPosition::kMinSource, \
+ TokenPosition::kMinSource); \
+ value = Smi::New(kTypedData##clazz##ViewCid); \
+ field.SetStaticValue(value, true); \
+ AddField(field);
+
+ CLASS_LIST_TYPED_DATA(ADD_SET_FIELD)
+#undef ADD_SET_FIELD
#undef CLASS_LIST_WITH_NULL
}
diff --git a/runtime/vm/symbols.h b/runtime/vm/symbols.h
index 23e3fde..38bfdf5 100644
--- a/runtime/vm/symbols.h
+++ b/runtime/vm/symbols.h
@@ -412,6 +412,7 @@
V(_UserTag, "_UserTag") \
V(Default, "Default") \
V(ClassID, "ClassID") \
+ V(getID, "getID") \
V(DartIsVM, "dart.isVM") \
V(stack, ":stack") \
V(stack_pointer, ":stack_pointer") \
diff --git a/sdk/lib/_internal/js_runtime/lib/convert_patch.dart b/sdk/lib/_internal/js_runtime/lib/convert_patch.dart
index 2d234a4..afb5062 100644
--- a/sdk/lib/_internal/js_runtime/lib/convert_patch.dart
+++ b/sdk/lib/_internal/js_runtime/lib/convert_patch.dart
@@ -496,3 +496,13 @@
return null;
}
}
+
+@patch
+int _scanOneByteCharacters(List<int> units, int from, int endIndex) {
+ final to = endIndex;
+ for (var i = from; i < to; i++) {
+ final unit = units[i];
+ if ((unit & _ONE_BYTE_LIMIT) != unit) return i - from;
+ }
+ return to - from;
+}
diff --git a/sdk/lib/convert/utf.dart b/sdk/lib/convert/utf.dart
index 567c8b2..eb92e2b 100644
--- a/sdk/lib/convert/utf.dart
+++ b/sdk/lib/convert/utf.dart
@@ -412,16 +412,6 @@
_expectedUnits = 0;
_extraUnits = 0;
- int scanOneByteCharacters(List<int> units, int from) {
- final to = endIndex;
- final mask = _ONE_BYTE_LIMIT;
- for (var i = from; i < to; i++) {
- final unit = units[i];
- if ((unit & mask) != unit) return i - from;
- }
- return to - from;
- }
-
void addSingleBytes(int from, int to) {
assert(from >= startIndex && from <= endIndex);
assert(to >= startIndex && to <= endIndex);
@@ -484,7 +474,7 @@
}
while (i < endIndex) {
- var oneBytes = scanOneByteCharacters(codeUnits, i);
+ var oneBytes = _scanOneByteCharacters(codeUnits, i, endIndex);
if (oneBytes > 0) {
_isFirstCharacter = false;
addSingleBytes(i, i + oneBytes);
@@ -545,3 +535,10 @@
}
}
}
+
+// Returns the number of bytes in [units] starting at offset [from] which have
+// the leftmost bit set to 0.
+//
+// To increase performance of this critical method we have a special variant of
+// it implemented in the VM's patch files, which is why we make it external.
+external int _scanOneByteCharacters(List<int> units, int from, int endIndex);
