[sparkplug] Allow short builtin calls only on machines with >= 4GB

... of physical memory, since builtins re-embedding comes with a memory
overhead.

Bug: v8:11527
Change-Id: I24b77c3ab63e1891bd4c6134c3f3456921cc2a01
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2784564Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73632}

src/baseline/arm64/baseline-assembler-arm64-inl.h

@@ -111,8 +111,8 @@ void BaselineAssembler::JumpIfNotSmi(Register value, Label* target,
 }
 
 void BaselineAssembler::CallBuiltin(Builtins::Name builtin) {
-  if (FLAG_short_builtin_calls) {
-    // Generate direct or pc-relative call.
+  if (masm()->options().short_builtin_calls) {
+    // Generate pc-relative call.
     __ CallBuiltin(builtin);
   } else {
     ScratchRegisterScope temps(this);
@@ -123,8 +123,8 @@ void BaselineAssembler::CallBuiltin(Builtins::Name builtin) {
 }
 
 void BaselineAssembler::TailCallBuiltin(Builtins::Name builtin) {
-  if (FLAG_short_builtin_calls) {
-    // Generate direct or pc-relative call.
+  if (masm()->options().short_builtin_calls) {
+    // Generate pc-relative call.
     __ TailCallBuiltin(builtin);
   } else {
     // The control flow integrity (CFI) feature allows us to "sign" code entry

src/baseline/x64/baseline-assembler-x64-inl.h

@@ -118,8 +118,8 @@ void BaselineAssembler::JumpIfNotSmi(Register value, Label* target,
 }
 
 void BaselineAssembler::CallBuiltin(Builtins::Name builtin) {
-  if (FLAG_short_builtin_calls) {
-    // Generate direct or pc-relative call.
+  if (masm()->options().short_builtin_calls) {
+    // Generate pc-relative call.
     __ CallBuiltin(builtin);
   } else {
     __ RecordCommentForOffHeapTrampoline(builtin);
@@ -129,8 +129,8 @@ void BaselineAssembler::CallBuiltin(Builtins::Name builtin) {
 }
 
 void BaselineAssembler::TailCallBuiltin(Builtins::Name builtin) {
-  if (FLAG_short_builtin_calls) {
-    // Generate direct or pc-relative jump.
+  if (masm()->options().short_builtin_calls) {
+    // Generate pc-relative jump.
     __ TailCallBuiltin(builtin);
   } else {
     __ RecordCommentForOffHeapTrampoline(builtin);

src/codegen/assembler.cc

@@ -74,7 +74,8 @@ AssemblerOptions AssemblerOptions::Default(Isolate* isolate) {
   options.code_range_start = code_range.begin();
 #endif
   options.short_builtin_calls =
-      FLAG_short_builtin_calls && !generating_embedded_builtin &&
+      isolate->is_short_builtin_calls_enabled() &&
+      !generating_embedded_builtin &&
       (options.code_range_start != kNullAddress) &&
       // Serialization of RUNTIME_ENTRY reloc infos is not supported yet.
       !serializer;

src/common/globals.h

@@ -108,6 +108,10 @@ STATIC_ASSERT(V8_DEFAULT_STACK_SIZE_KB* KB +
 #endif
 #endif
 
+// This constant is used for detecting whether the machine has >= 4GB of
+// physical memory by checking the max old space size.
+const size_t kShortBuiltinCallsOldSpaceSizeThreshold = size_t{2} * GB;
+
 // Determine whether dict mode prototypes feature is enabled.
 #ifdef V8_DICT_MODE_PROTOTYPES
 #define V8_DICT_MODE_PROTOTYPES_BOOL true

src/execution/isolate.cc

@@ -3383,7 +3383,7 @@ void Isolate::CreateAndSetEmbeddedBlob() {
 }
 
 void Isolate::MaybeRemapEmbeddedBuiltinsIntoCodeRange() {
-  if (!FLAG_short_builtin_calls || !RequiresCodeRange()) return;
+  if (!is_short_builtin_calls_enabled() || !RequiresCodeRange()) return;
 
   CHECK_NOT_NULL(embedded_blob_code_);
   CHECK_NE(embedded_blob_code_size_, 0);
@@ -3399,7 +3399,7 @@ void Isolate::TearDownEmbeddedBlob() {
   // Nothing to do in case the blob is embedded into the binary or unset.
   if (StickyEmbeddedBlobCode() == nullptr) return;
 
-  if (!FLAG_short_builtin_calls) {
+  if (!is_short_builtin_calls_enabled()) {
     CHECK_EQ(embedded_blob_code(), StickyEmbeddedBlobCode());
     CHECK_EQ(embedded_blob_data(), StickyEmbeddedBlobData());
   }
@@ -3544,6 +3544,13 @@ bool Isolate::Init(SnapshotData* startup_snapshot_data,
   ReadOnlyHeap::SetUp(this, read_only_snapshot_data, can_rehash);
   heap_.SetUpSpaces();
 
+  if (V8_SHORT_BUILTIN_CALLS_BOOL && FLAG_short_builtin_calls) {
+    // Check if the system has more than 4GB of physical memory by comaring
+    // the old space size with respective threshod value.
+    is_short_builtin_calls_enabled_ =
+        heap_.MaxOldGenerationSize() >= kShortBuiltinCallsOldSpaceSizeThreshold;
+  }
+
   // Create LocalIsolate/LocalHeap for the main thread and set state to Running.
   main_thread_local_isolate_.reset(new LocalIsolate(this, ThreadKind::kMain));
   main_thread_local_heap()->Unpark();

src/execution/isolate.h

@@ -1498,6 +1498,11 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   const uint8_t* embedded_blob_data() const;
   uint32_t embedded_blob_data_size() const;
 
+  // Returns true if short bultin calls optimization is enabled for the Isolate.
+  bool is_short_builtin_calls_enabled() const {
+    return V8_SHORT_BUILTIN_CALLS_BOOL && is_short_builtin_calls_enabled_;
+  }
+
   void set_array_buffer_allocator(v8::ArrayBuffer::Allocator* allocator) {
     array_buffer_allocator_ = allocator;
   }
@@ -1920,9 +1925,8 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
   // True if this isolate was initialized from a snapshot.
   bool initialized_from_snapshot_ = false;
 
-  // TODO(ishell): remove
-  // True if ES2015 tail call elimination feature is enabled.
-  bool is_tail_call_elimination_enabled_ = true;
+  // True if short bultin calls optimization is enabled.
+  bool is_short_builtin_calls_enabled_ = false;
 
   // True if the isolate is in background. This flag is used
   // to prioritize between memory usage and latency.

src/flags/flag-definitions.h

@@ -1540,13 +1540,15 @@ DEFINE_BOOL(experimental_flush_embedded_blob_icache, false,
 #undef FLAG
 #if V8_SHORT_BUILTIN_CALLS
 #define FLAG FLAG_FULL
+#define V8_SHORT_BUILTIN_CALLS_BOOL true
 #else
 #define FLAG FLAG_READONLY
+#define V8_SHORT_BUILTIN_CALLS_BOOL false
 #endif
 
 DEFINE_BOOL(short_builtin_calls, false,
             "Put embedded builtins code into the code range for shorter "
-            "builtin calls/jumps")
+            "builtin calls/jumps if system has >=4GB memory")
 
 #undef FLAG
 #define FLAG FLAG_FULL

src/objects/code.cc

@@ -154,6 +154,9 @@ Address Code::OffHeapInstructionStart() const {
   // GetIsolateFromWritableObject(*this) works for both read-only and writable
   // objects here because short builtin calls feature requires pointer
   // compression.
+  // We don't have to check the Isolate::is_short_builtin_calls_enabled() value
+  // because if the short builtin calls wasn't actually enabled because of not
+  // enough memory, the FromBlob(isolate) would still be the correct one to use.
   EmbeddedData d =
       FLAG_short_builtin_calls
           ? EmbeddedData::FromBlob(GetIsolateFromWritableObject(*this))
@@ -170,6 +173,9 @@ Address Code::OffHeapInstructionEnd() const {
   // GetIsolateFromWritableObject(*this) works for both read-only and writable
   // objects here because short builtin calls feature requires pointer
   // compression.
+  // We don't have to check the Isolate::is_short_builtin_calls_enabled() value
+  // because if the short builtin calls wasn't actually enabled because of not
+  // enough memory, the FromBlob(isolate) would still be the correct one to use.
   EmbeddedData d =
       FLAG_short_builtin_calls
           ? EmbeddedData::FromBlob(GetIsolateFromWritableObject(*this))

src/objects/code.h

@@ -150,9 +150,10 @@ class Code : public HeapObject {
   inline Address InstructionEnd() const;
   V8_EXPORT_PRIVATE Address OffHeapInstructionEnd() const;
 
-  // When builtins un-embedding (FLAG_short_builtin_calls) is enabled both
-  // embedded and un-embedded builtins might be exeuted and thus two kinds of
-  // |pc|s might appear on the stack.
+  // When builtins un-embedding is enabled for the Isolate
+  // (see Isolate::is_short_builtin_calls_enabled()) then both embedded and
+  // un-embedded builtins might be exeuted and thus two kinds of |pc|s might
+  // appear on the stack.
   // Unlike the paremeterless versions of the functions above the below variants
   // ensure that the instruction start correspond to the given |pc| value.
   // Thus for off-heap trampoline Code objects the result might be the

src/snapshot/embedded/embedded-data.cc

@@ -51,7 +51,8 @@ bool InstructionStream::PcIsOffHeap(Isolate* isolate, Address pc) {
   DCHECK_NOT_NULL(Isolate::CurrentEmbeddedBlobCode());
 
   if (EmbeddedData::FromBlob(isolate).IsInCodeRange(pc)) return true;
-  return FLAG_short_builtin_calls && EmbeddedData::FromBlob().IsInCodeRange(pc);
+  return isolate->is_short_builtin_calls_enabled() &&
+         EmbeddedData::FromBlob().IsInCodeRange(pc);
 }
 
 // static
@@ -68,7 +69,7 @@ bool InstructionStream::TryGetAddressForHashing(Isolate* isolate,
     return true;
   }
 
-  if (FLAG_short_builtin_calls) {
+  if (isolate->is_short_builtin_calls_enabled()) {
     d = EmbeddedData::FromBlob();
     if (d.IsInCodeRange(address)) {
       *hashable_address = d.AddressForHashing(address);
@@ -88,7 +89,8 @@ Builtins::Name InstructionStream::TryLookupCode(Isolate* isolate,
   Builtins::Name builtin =
       i::TryLookupCode(EmbeddedData::FromBlob(isolate), address);
 
-  if (FLAG_short_builtin_calls && !Builtins::IsBuiltinId(builtin)) {
+  if (isolate->is_short_builtin_calls_enabled() &&
+      !Builtins::IsBuiltinId(builtin)) {
     builtin = i::TryLookupCode(EmbeddedData::FromBlob(), address);
   }
   return builtin;

src/snapshot/embedded/embedded-data.h

@@ -72,18 +72,19 @@ class EmbeddedData final {
     return (start <= pc) && (pc < start + code_size_);
   }
 
-  // When FLAG_short_builtin_calls is enabled, there will be two builtins
-  // instruction streams executed: the embedded one and the one un-embedded into
-  // the per-Isolate code range. In most of the cases, the per-Isolate
-  // instructions will be used but in some cases (like builtin calls from Wasm)
-  // the embedded instruction stream could be used.
+  // When short builtin calls optimization is enabled for the Isolate, there
+  // will be two builtins instruction streams executed: the embedded one and
+  // the one un-embedded into the per-Isolate code range. In most of the cases,
+  // the per-Isolate instructions will be used but in some cases (like builtin
+  // calls from Wasm) the embedded instruction stream could be used.
   // If the requested PC belongs to the embedded code blob - it'll be returned,
   // and the per-Isolate blob otherwise.
   // See http://crbug.com/v8/11527 for details.
   inline static EmbeddedData GetEmbeddedDataForPC(Isolate* isolate,
                                                   Address maybe_builtin_pc) {
     EmbeddedData d = EmbeddedData::FromBlob(isolate);
-    if (FLAG_short_builtin_calls && !d.IsInCodeRange(maybe_builtin_pc)) {
+    if (isolate->is_short_builtin_calls_enabled() &&
+        !d.IsInCodeRange(maybe_builtin_pc)) {
       EmbeddedData global_d = EmbeddedData::FromBlob();
       // If the pc does not belong to the embedded code blob we should be using
       // the un-embedded one.

test/cctest/test-code-pages.cc

@@ -106,7 +106,7 @@ TEST(CodeRangeCorrectContents) {
   CHECK(PagesHasExactPage(
       pages, reinterpret_cast<Address>(i_isolate->CurrentEmbeddedBlobCode()),
       i_isolate->CurrentEmbeddedBlobCodeSize()));
-  if (FLAG_short_builtin_calls) {
+  if (i_isolate->is_short_builtin_calls_enabled()) {
     // In this case embedded blob code must be included via code_range.
     CHECK(PagesContainsRange(
         pages, reinterpret_cast<Address>(i_isolate->embedded_blob_code()),

test/cctest/test-code-stub-assembler.cc

@@ -2453,6 +2453,33 @@ TEST(IsDebugActive) {
   *debug_is_active = false;
 }
 
+// Ensure that the kShortBuiltinCallsOldSpaceSizeThreshold constant can be used
+// for detecting whether the machine has >= 4GB of physical memory by checking
+// the max old space size.
+TEST(ShortBuiltinCallsThreshold) {
+  if (!V8_SHORT_BUILTIN_CALLS_BOOL) return;
+
+  const uint64_t kPhysicalMemoryThreshold = size_t{4} * GB;
+
+  size_t heap_size, old, young;
+
+  // If the physical memory is < kPhysicalMemoryThreshold then the old space
+  // size must be below the kShortBuiltinCallsOldSpaceThreshold.
+  heap_size = Heap::HeapSizeFromPhysicalMemory(kPhysicalMemoryThreshold - MB);
+  i::Heap::GenerationSizesFromHeapSize(heap_size, &young, &old);
+  CHECK_LT(old, kShortBuiltinCallsOldSpaceSizeThreshold);
+
+  // If the physical memory is >= kPhysicalMemoryThreshold then the old space
+  // size must be below the kShortBuiltinCallsOldSpaceThreshold.
+  heap_size = Heap::HeapSizeFromPhysicalMemory(kPhysicalMemoryThreshold);
+  i::Heap::GenerationSizesFromHeapSize(heap_size, &young, &old);
+  CHECK_GE(old, kShortBuiltinCallsOldSpaceSizeThreshold);
+
+  heap_size = Heap::HeapSizeFromPhysicalMemory(kPhysicalMemoryThreshold + MB);
+  i::Heap::GenerationSizesFromHeapSize(heap_size, &young, &old);
+  CHECK_GE(old, kShortBuiltinCallsOldSpaceSizeThreshold);
+}
+
 TEST(CallBuiltin) {
   Isolate* isolate(CcTest::InitIsolateOnce());