[coverage] Reduce IncBlockCounter overhead

When collecting JS block coverage, we track block execution counts on
so-called CoverageInfo objects. Generated bytecode and native code
contains inlined snippets of code to increment the appropriate
counters.

These used to be implemented as calls to the IncBlockCounter runtime
function. Each call incurred the entire CEntry overhead.

This CL reduces that overhead by moving logic over into a new
IncBlockCounter TFS builtin. The builtin is called directly from
bytecode, and lowered to the same builtin call for optimized code.

Drive-by: Tweak CoverageInfo layout to generate faster code.

Tbr: jarin@chromium.org
Bug: v8:9149, v8:6000
Change-Id: I2d7cb0db649edf7c56b5ef5a4683d27b1c34605c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1571420Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60981}

BUILD.gn

@@ -938,6 +938,7 @@ torque_files = [
   "src/builtins/collections.tq",
   "src/builtins/data-view.tq",
   "src/builtins/extras-utils.tq",
+  "src/builtins/internal-coverage.tq",
   "src/builtins/iterator.tq",
   "src/builtins/object-fromentries.tq",
   "src/builtins/proxy.tq",
@@ -988,6 +989,7 @@ torque_namespaces = [
   "data-view",
   "extras-utils",
   "growable-fixed-array",
+  "internal-coverage",
   "iterator",
   "object",
   "proxy",

src/builtins/base.tq

@@ -340,7 +340,6 @@ extern class Script extends Struct {
   source_mapping_url: Object;
   host_defined_options: Object;
 }
-type DebugInfo extends HeapObject;
 
 extern class EmbedderDataArray extends HeapObject { length: Smi; }
 
@@ -1033,6 +1032,18 @@ extern class AccessorPair extends Struct {
 
 extern class BreakPoint extends Tuple2 {}
 extern class BreakPointInfo extends Tuple2 {}
+extern class CoverageInfo extends FixedArray {}
+
+extern class DebugInfo extends Struct {
+  shared_function_info: HeapObject;
+  debugger_hints: Smi;
+  script: Undefined | Script;
+  original_bytecode_array: Undefined | BytecodeArray;
+  debug_bytecode_array: Undefined | BytecodeArray;
+  break_points: FixedArray;
+  flags: Smi;
+  coverage_info: CoverageInfo | Undefined;
+}
 
 extern class WasmModuleObject extends JSObject {
   native_module: Foreign;
@@ -1654,6 +1665,21 @@ Cast<JSFunction>(implicit context: Context)(o: HeapObject): JSFunction
   goto CastError;
 }
 
+extern macro IsDebugInfo(HeapObject): bool;
+Cast<DebugInfo>(implicit context: Context)(o: HeapObject): DebugInfo
+    labels CastError {
+  if (IsDebugInfo(o)) return %RawDownCast<DebugInfo>(o);
+  goto CastError;
+}
+
+extern macro IsCoverageInfo(HeapObject): bool;
+Cast<CoverageInfo>(implicit context: Context)(o: HeapObject): CoverageInfo
+    labels CastError {
+  // TODO(jgruber): Assign an instance type.
+  if (IsFixedArray(o)) return %RawDownCast<CoverageInfo>(o);
+  goto CastError;
+}
+
 extern macro AllocateHeapNumberWithValue(float64): HeapNumber;
 extern macro ChangeInt32ToTagged(int32): Number;
 extern macro ChangeUint32ToTagged(uint32): Number;

src/builtins/internal-coverage.tq

+// Copyright 2019 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include 'src/builtins/builtins-regexp-gen.h'
+
+namespace internal_coverage {
+
+  const kHasCoverageInfo:
+      constexpr int31 generates 'DebugInfo::kHasCoverageInfo';
+
+  const kFirstSlotIndex:
+      constexpr int31 generates 'CoverageInfo::kFirstSlotIndex';
+  const kSlotBlockCountIndex:
+      constexpr int31 generates 'CoverageInfo::kSlotBlockCountIndex';
+  const kSlotIndexCountLog2:
+      constexpr int31 generates 'CoverageInfo::kSlotIndexCountLog2';
+  const kSlotIndexCountMask:
+      constexpr int31 generates 'CoverageInfo::kSlotIndexCountMask';
+
+  macro GetCoverageInfo(implicit context: Context)(function: JSFunction):
+      CoverageInfo labels IfNoCoverageInfo {
+    const shared: SharedFunctionInfo = function.shared_function_info;
+    const debugInfo = Cast<DebugInfo>(shared.script_or_debug_info)
+        otherwise goto IfNoCoverageInfo;
+
+    if ((debugInfo.flags & kHasCoverageInfo) == 0) goto IfNoCoverageInfo;
+    return UnsafeCast<CoverageInfo>(debugInfo.coverage_info);
+  }
+
+  macro SlotCount(coverageInfo: CoverageInfo): Smi {
+    assert(kFirstSlotIndex == 0);  // Otherwise we'd have to consider it below.
+    assert(kFirstSlotIndex == (coverageInfo.length & kSlotIndexCountMask));
+    return coverageInfo.length >> kSlotIndexCountLog2;
+  }
+
+  macro FirstIndexForSlot(implicit context: Context)(slot: Smi): Smi {
+    assert(kFirstSlotIndex == 0);  // Otherwise we'd have to consider it below.
+    return slot << kSlotIndexCountLog2;
+  }
+
+  macro IncrementBlockCount(implicit context: Context)(
+      coverageInfo: CoverageInfo, slot: Smi) {
+    assert(slot < SlotCount(coverageInfo));
+    const slotStart: Smi = FirstIndexForSlot(slot);
+    const index: Smi = slotStart + kSlotBlockCountIndex;
+    coverageInfo.objects[index] =
+        UnsafeCast<Smi>(coverageInfo.objects[index]) + 1;
+  }
+
+  builtin IncBlockCounter(implicit context: Context)(
+      function: JSFunction, coverageArraySlotIndex: Smi): Object {
+    // It's quite possible that a function contains IncBlockCounter bytecodes,
+    // but no coverage info exists. This happens e.g. by selecting the
+    // best-effort coverage collection mode, which triggers deletion of all
+    // coverage infos in order to avoid memory leaks.
+
+    const coverageInfo: CoverageInfo =
+        GetCoverageInfo(function) otherwise return Undefined;
+    IncrementBlockCount(coverageInfo, coverageArraySlotIndex);
+    return Undefined;
+  }
+
+}  // namespace internal_coverage

src/code-stub-assembler.cc

@@ -5917,6 +5917,10 @@ TNode<BoolT> CodeStubAssembler::IsCallableMap(SloppyTNode<Map> map) {
   return IsSetWord32<Map::IsCallableBit>(LoadMapBitField(map));
 }
 
+TNode<BoolT> CodeStubAssembler::IsDebugInfo(TNode<HeapObject> object) {
+  return HasInstanceType(object, DEBUG_INFO_TYPE);
+}
+
 TNode<BoolT> CodeStubAssembler::IsDeprecatedMap(SloppyTNode<Map> map) {
   CSA_ASSERT(this, IsMap(map));
   return IsSetWord32<Map::IsDeprecatedBit>(LoadMapBitField3(map));

src/code-stub-assembler.h

@@ -2146,6 +2146,7 @@ class V8_EXPORT_PRIVATE CodeStubAssembler
   TNode<BoolT> IsConsStringInstanceType(SloppyTNode<Int32T> instance_type);
   TNode<BoolT> IsConstructorMap(SloppyTNode<Map> map);
   TNode<BoolT> IsConstructor(SloppyTNode<HeapObject> object);
+  TNode<BoolT> IsDebugInfo(TNode<HeapObject> object);
   TNode<BoolT> IsDeprecatedMap(SloppyTNode<Map> map);
   TNode<BoolT> IsNameDictionary(SloppyTNode<HeapObject> object);
   TNode<BoolT> IsGlobalDictionary(SloppyTNode<HeapObject> object);

src/compiler/bytecode-graph-builder.cc

@@ -2817,7 +2817,9 @@ void BytecodeGraphBuilder::VisitIncBlockCounter() {
   Node* coverage_array_slot =
       jsgraph()->Constant(bytecode_iterator().GetIndexOperand(0));
 
-  const Operator* op = javascript()->CallRuntime(Runtime::kIncBlockCounter);
+  // Lowered by js-intrinsic-lowering to call Builtins::kIncBlockCounter.
+  const Operator* op =
+      javascript()->CallRuntime(Runtime::kInlineIncBlockCounter);
 
   NewNode(op, closure, coverage_array_slot);
 }

src/compiler/js-intrinsic-lowering.cc

@@ -76,6 +76,8 @@ Reduction JSIntrinsicLowering::Reduce(Node* node) {
       return ReduceToString(node);
     case Runtime::kInlineCall:
       return ReduceCall(node);
+    case Runtime::kInlineIncBlockCounter:
+      return ReduceIncBlockCounter(node);
     default:
       break;
   }
@@ -301,6 +303,14 @@ Reduction JSIntrinsicLowering::ReduceCall(Node* node) {
   return Changed(node);
 }
 
+Reduction JSIntrinsicLowering::ReduceIncBlockCounter(Node* node) {
+  DCHECK(!Linkage::NeedsFrameStateInput(Runtime::kIncBlockCounter));
+  DCHECK(!Linkage::NeedsFrameStateInput(Runtime::kInlineIncBlockCounter));
+  return Change(node,
+                Builtins::CallableFor(isolate(), Builtins::kIncBlockCounter), 0,
+                kDoesNotNeedFrameState);
+}
+
 Reduction JSIntrinsicLowering::Change(Node* node, const Operator* op, Node* a,
                                       Node* b) {
   RelaxControls(node);
@@ -336,19 +346,21 @@ Reduction JSIntrinsicLowering::Change(Node* node, const Operator* op, Node* a,
   return Changed(node);
 }
 
-
 Reduction JSIntrinsicLowering::Change(Node* node, Callable const& callable,
-                                      int stack_parameter_count) {
+                                      int stack_parameter_count,
+                                      enum FrameStateFlag frame_state_flag) {
+  CallDescriptor::Flags flags = frame_state_flag == kNeedsFrameState
+                                    ? CallDescriptor::kNeedsFrameState
+                                    : CallDescriptor::kNoFlags;
   auto call_descriptor = Linkage::GetStubCallDescriptor(
-      graph()->zone(), callable.descriptor(), stack_parameter_count,
-      CallDescriptor::kNeedsFrameState, node->op()->properties());
+      graph()->zone(), callable.descriptor(), stack_parameter_count, flags,
+      node->op()->properties());
   node->InsertInput(graph()->zone(), 0,
                     jsgraph()->HeapConstant(callable.code()));
   NodeProperties::ChangeOp(node, common()->Call(call_descriptor));
   return Changed(node);
 }
 
-
 Graph* JSIntrinsicLowering::graph() const { return jsgraph()->graph(); }
 
 

src/compiler/js-intrinsic-lowering.h

@@ -61,14 +61,21 @@ class V8_EXPORT_PRIVATE JSIntrinsicLowering final
   Reduction ReduceToObject(Node* node);
   Reduction ReduceToString(Node* node);
   Reduction ReduceCall(Node* node);
+  Reduction ReduceIncBlockCounter(Node* node);
 
   Reduction Change(Node* node, const Operator* op);
   Reduction Change(Node* node, const Operator* op, Node* a, Node* b);
   Reduction Change(Node* node, const Operator* op, Node* a, Node* b, Node* c);
   Reduction Change(Node* node, const Operator* op, Node* a, Node* b, Node* c,
                    Node* d);
+
+  enum FrameStateFlag {
+    kNeedsFrameState,
+    kDoesNotNeedFrameState,
+  };
   Reduction Change(Node* node, Callable const& callable,
-                   int stack_parameter_count);
+                   int stack_parameter_count,
+                   enum FrameStateFlag frame_state_flag = kNeedsFrameState);
 
   Graph* graph() const;
   JSGraph* jsgraph() const { return jsgraph_; }

src/compiler/linkage.cc

@@ -197,6 +197,7 @@ bool Linkage::NeedsFrameStateInput(Runtime::FunctionId function) {
 
     // Some inline intrinsics are also safe to call without a FrameState.
     case Runtime::kInlineCreateIterResultObject:
+    case Runtime::kInlineIncBlockCounter:
     case Runtime::kInlineGeneratorClose:
     case Runtime::kInlineGeneratorGetResumeMode:
     case Runtime::kInlineCreateJSGeneratorObject:

src/interpreter/interpreter-generator.cc

@@ -2961,7 +2961,8 @@ IGNITION_HANDLER(IncBlockCounter, InterpreterAssembler) {
   Node* coverage_array_slot = BytecodeOperandIdxSmi(0);
   Node* context = GetContext();
 
-  CallRuntime(Runtime::kIncBlockCounter, context, closure, coverage_array_slot);
+  CallBuiltin(Builtins::kIncBlockCounter, context, closure,
+              coverage_array_slot);
 
   Dispatch();
 }

src/objects/debug-objects-inl.h

@@ -37,7 +37,7 @@ ACCESSORS(DebugInfo, script, Object, kScriptOffset)
 ACCESSORS(DebugInfo, original_bytecode_array, Object,
           kOriginalBytecodeArrayOffset)
 ACCESSORS(DebugInfo, debug_bytecode_array, Object, kDebugBytecodeArrayOffset)
-ACCESSORS(DebugInfo, break_points, FixedArray, kBreakPointsStateOffset)
+ACCESSORS(DebugInfo, break_points, FixedArray, kBreakPointsOffset)
 ACCESSORS(DebugInfo, coverage_info, Object, kCoverageInfoOffset)
 
 BIT_FIELD_ACCESSORS(DebugInfo, debugger_hints, side_effect_state,

src/objects/debug-objects.h

@@ -168,21 +168,9 @@ class DebugInfo : public Struct {
   DECL_PRINTER(DebugInfo)
   DECL_VERIFIER(DebugInfo)
 
-// Layout description.
-#define DEBUG_INFO_FIELDS(V)                   \
-  V(kSharedFunctionInfoOffset, kTaggedSize)    \
-  V(kDebuggerHintsOffset, kTaggedSize)         \
-  V(kScriptOffset, kTaggedSize)                \
-  V(kOriginalBytecodeArrayOffset, kTaggedSize) \
-  V(kDebugBytecodeArrayOffset, kTaggedSize)    \
-  V(kBreakPointsStateOffset, kTaggedSize)      \
-  V(kFlagsOffset, kTaggedSize)                 \
-  V(kCoverageInfoOffset, kTaggedSize)          \
-  /* Total size. */                            \
-  V(kSize, 0)
-
-  DEFINE_FIELD_OFFSET_CONSTANTS(Struct::kHeaderSize, DEBUG_INFO_FIELDS)
-#undef DEBUG_INFO_FIELDS
+  // Layout description.
+  DEFINE_FIELD_OFFSET_CONSTANTS(Struct::kHeaderSize,
+                                TORQUE_GENERATED_DEBUG_INFO_FIELDS)
 
   static const int kEstimatedNofBreakPointsInFunction = 4;
 
@@ -247,11 +235,6 @@ class CoverageInfo : public FixedArray {
   // Print debug info.
   void Print(std::unique_ptr<char[]> function_name);
 
- private:
-  static int FirstIndexForSlot(int slot_index) {
-    return kFirstSlotIndex + slot_index * kSlotIndexCount;
-  }
-
   static const int kFirstSlotIndex = 0;
 
   // Each slot is assigned a group of indices starting at kFirstSlotIndex.
@@ -259,7 +242,17 @@ class CoverageInfo : public FixedArray {
   static const int kSlotStartSourcePositionIndex = 0;
   static const int kSlotEndSourcePositionIndex = 1;
   static const int kSlotBlockCountIndex = 2;
-  static const int kSlotIndexCount = 3;
+  static const int kSlotPaddingIndex = 3;  // Padding to make the index count 4.
+  static const int kSlotIndexCount = 4;
+
+  static const int kSlotIndexCountLog2 = 2;
+  static const int kSlotIndexCountMask = (kSlotIndexCount - 1);
+  STATIC_ASSERT(1 << kSlotIndexCountLog2 == kSlotIndexCount);
+
+ private:
+  static int FirstIndexForSlot(int slot_index) {
+    return kFirstSlotIndex + slot_index * kSlotIndexCount;
+  }
 
   OBJECT_CONSTRUCTORS(CoverageInfo, FixedArray);
 };

src/runtime/runtime-debug.cc

@@ -737,23 +737,7 @@ RUNTIME_FUNCTION(Runtime_DebugToggleBlockCoverage) {
 }
 
 RUNTIME_FUNCTION(Runtime_IncBlockCounter) {
-  SealHandleScope scope(isolate);
-  DCHECK_EQ(2, args.length());
-  CONVERT_ARG_CHECKED(JSFunction, function, 0);
-  CONVERT_SMI_ARG_CHECKED(coverage_array_slot_index, 1);
-
-  // It's quite possible that a function contains IncBlockCounter bytecodes, but
-  // no coverage info exists. This happens e.g. by selecting the best-effort
-  // coverage collection mode, which triggers deletion of all coverage infos in
-  // order to avoid memory leaks.
-
-  SharedFunctionInfo shared = function->shared();
-  if (shared->HasCoverageInfo()) {
-    CoverageInfo coverage_info = shared->GetCoverageInfo();
-    coverage_info->IncrementBlockCount(coverage_array_slot_index);
-  }
-
-  return ReadOnlyRoots(isolate).undefined_value();
+  UNREACHABLE();  // Never called. See the IncBlockCounter builtin instead.
 }
 
 RUNTIME_FUNCTION(Runtime_DebugAsyncFunctionEntered) {

src/runtime/runtime.h

@@ -137,7 +137,7 @@ namespace internal {
   F(GetGeneratorScopeDetails, 2, 1)             \
   F(GetHeapUsage, 0, 1)                         \
   F(HandleDebuggerStatement, 0, 1)              \
-  F(IncBlockCounter, 2, 1)                      \
+  I(IncBlockCounter, 2, 1)                      \
   F(IsBreakOnException, 1, 1)                   \
   F(ScheduleBreak, 0, 1)                        \
   F(ScriptLocationFromLine2, 4, 1)              \