[wasm] Store protected instructions in an OwnedVector

We currently store the protected instructions per code object in a
{std::unique_ptr<std::vector<ProtectedInstructionData>>}. This wastes
memory, because it requires two heap allocations, plus the vector might
over-allocate (and it currently does, because it is filled dynamically
during compilation).
This CL changes that to store the protected instructions in an
{OwnedVector}. This requires one copy after generating the list of
{ProtectedInstructionData} in an {std::vector} during compilation, but
saves memory afterwards.

R=mstarzinger@chromium.org

Bug: chromium:856938
Change-Id: Ie290a17dc32f27fbbfe0c000a52297181c954550
Reviewed-on: https://chromium-review.googlesource.com/1116701Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54052}

src/compiler/pipeline.cc

@@ -1051,7 +1051,7 @@ PipelineWasmCompilationJob::Status PipelineWasmCompilationJob::FinalizeJobImpl(
       code_desc, code_generator->frame()->GetTotalFrameSlotCount(),
       data_.wasm_function_index(), code_generator->GetSafepointTableOffset(),
       code_generator->GetHandlerTableOffset(),
-      data_.wasm_compilation_data()->ReleaseProtectedInstructions(),
+      data_.wasm_compilation_data()->GetProtectedInstructions(),
       code_generator->GetSourcePositionTable(), wasm::WasmCode::kTurbofan);
 
   if (!code) return FAILED;

src/compiler/wasm-compiler.cc

@@ -4999,18 +4999,6 @@ MaybeHandle<Code> CompileCWasmEntry(Isolate* isolate, wasm::FunctionSig* sig) {
   return code;
 }
 
-WasmCompilationData::WasmCompilationData(
-    wasm::RuntimeExceptionSupport runtime_exception_support)
-    : protected_instructions_(
-          new std::vector<trap_handler::ProtectedInstructionData>()),
-      runtime_exception_support_(runtime_exception_support) {}
-
-void WasmCompilationData::AddProtectedInstruction(uint32_t instr_offset,
-                                                  uint32_t landing_offset) {
-  protected_instructions_->emplace_back(
-      trap_handler::ProtectedInstructionData{instr_offset, landing_offset});
-}
-
 TurbofanWasmCompilationUnit::TurbofanWasmCompilationUnit(
     wasm::WasmCompilationUnit* wasm_unit)
     : wasm_unit_(wasm_unit),

src/compiler/wasm-compiler.h

@@ -52,13 +52,17 @@ namespace compiler {
 class WasmCompilationData {
  public:
   explicit WasmCompilationData(
-      wasm::RuntimeExceptionSupport runtime_exception_support);
+      wasm::RuntimeExceptionSupport runtime_exception_support)
+      : runtime_exception_support_(runtime_exception_support) {}
 
-  void AddProtectedInstruction(uint32_t instr_offset, uint32_t landing_offset);
+  void AddProtectedInstruction(uint32_t instr_offset, uint32_t landing_offset) {
+    protected_instructions_.push_back({instr_offset, landing_offset});
+  }
 
-  std::unique_ptr<std::vector<trap_handler::ProtectedInstructionData>>
-  ReleaseProtectedInstructions() {
-    return std::move(protected_instructions_);
+  OwnedVector<trap_handler::ProtectedInstructionData>
+  GetProtectedInstructions() {
+    return OwnedVector<trap_handler::ProtectedInstructionData>::Of(
+        protected_instructions_);
   }
 
   wasm::RuntimeExceptionSupport runtime_exception_support() const {
@@ -66,8 +70,7 @@ class WasmCompilationData {
   }
 
  private:
-  std::unique_ptr<std::vector<trap_handler::ProtectedInstructionData>>
-      protected_instructions_;
+  std::vector<trap_handler::ProtectedInstructionData> protected_instructions_;
 
   // See ModuleEnv::runtime_exception_support_.
   wasm::RuntimeExceptionSupport runtime_exception_support_;

src/source-position-table.cc

@@ -176,8 +176,7 @@ OwnedVector<byte> SourcePositionTableBuilder::ToSourcePositionTableVector() {
   if (bytes_.empty()) return OwnedVector<byte>();
   DCHECK(!Omit());
 
-  OwnedVector<byte> table = OwnedVector<byte>::New(bytes_.size());
-  MemCopy(table.start(), bytes_.data(), bytes_.size());
+  OwnedVector<byte> table = OwnedVector<byte>::Of(bytes_);
 
 #ifdef ENABLE_SLOW_DCHECKS
   // Brute force testing: Record all positions and decode

src/vector.h

@@ -5,8 +5,9 @@
 #ifndef V8_VECTOR_H_
 #define V8_VECTOR_H_
 
-#include <string.h>
 #include <algorithm>
+#include <cstring>
+#include <iterator>
 
 #include "src/allocation.h"
 #include "src/checks.h"
@@ -210,9 +211,24 @@ class OwnedVector {
 
   // Allocates a new vector of the specified size via the default allocator.
   static OwnedVector<T> New(size_t size) {
+    if (size == 0) return {};
     return OwnedVector<T>(std::unique_ptr<T[]>(new T[size]), size);
   }
 
+  // Allocates a new vector containing the specified collection of values.
+  // {Iterator} is the common type of {std::begin} and {std::end} called on a
+  // {const U&}. This function is only instantiable if that type exists.
+  template <typename U, typename Iterator = typename std::common_type<
+                            decltype(std::begin(std::declval<const U&>())),
+                            decltype(std::end(std::declval<const U&>()))>::type>
+  static OwnedVector<T> Of(const U& collection) {
+    Iterator begin = std::begin(collection);
+    Iterator end = std::end(collection);
+    OwnedVector<T> vec = New(std::distance(begin, end));
+    std::copy(begin, end, vec.start());
+    return vec;
+  }
+
  private:
   std::unique_ptr<T[]> data_;
   size_t length_ = 0;

src/wasm/baseline/liftoff-compiler.cc

@@ -1830,13 +1830,11 @@ bool LiftoffCompilationUnit::ExecuteCompilation() {
       compiler::GetWasmCallDescriptor(&zone, wasm_unit_->func_body_.sig);
   base::Optional<TimedHistogramScope> liftoff_compile_time_scope(
       base::in_place, wasm_unit_->counters_->liftoff_compile_time());
-  DCHECK(!protected_instructions_);
-  protected_instructions_.reset(
-      new std::vector<trap_handler::ProtectedInstructionData>());
+  DCHECK(protected_instructions_.empty());
   wasm::WasmFullDecoder<wasm::Decoder::kValidate, wasm::LiftoffCompiler>
       decoder(&zone, module, wasm_unit_->func_body_, &asm_, call_descriptor,
               wasm_unit_->env_, &source_position_table_builder_,
-              protected_instructions_.get(), &zone);
+              &protected_instructions_, &zone);
   decoder.Decode();
   liftoff_compile_time_scope.reset();
   if (!decoder.interface().ok()) {
@@ -1861,7 +1859,7 @@ bool LiftoffCompilationUnit::ExecuteCompilation() {
   // Record the memory cost this unit places on the system until
   // it is finalized.
   wasm_unit_->memory_cost_ =
-      asm_.pc_offset() + protected_instructions_->size() *
+      asm_.pc_offset() + protected_instructions_.size() *
                              sizeof(trap_handler::ProtectedInstructionData);
 
   safepoint_table_offset_ = decoder.interface().GetSafepointTableOffset();
@@ -1876,10 +1874,12 @@ wasm::WasmCode* LiftoffCompilationUnit::FinishCompilation(
 
   OwnedVector<byte> source_positions =
       source_position_table_builder_.ToSourcePositionTableVector();
-
+  auto protected_instructions_copy =
+      OwnedVector<trap_handler::ProtectedInstructionData>::Of(
+          protected_instructions_);
   wasm::WasmCode* code = wasm_unit_->native_module_->AddCode(
       desc, asm_.GetTotalFrameSlotCount(), wasm_unit_->func_index_,
-      safepoint_table_offset_, 0, std::move(protected_instructions_),
+      safepoint_table_offset_, 0, std::move(protected_instructions_copy),
       std::move(source_positions), wasm::WasmCode::kLiftoff);
 
   return code;

src/wasm/baseline/liftoff-compiler.h

@@ -29,8 +29,7 @@ class LiftoffCompilationUnit final {
   wasm::LiftoffAssembler asm_;
   int safepoint_table_offset_;
   SourcePositionTableBuilder source_position_table_builder_;
-  std::unique_ptr<std::vector<trap_handler::ProtectedInstructionData>>
-      protected_instructions_;
+  std::vector<trap_handler::ProtectedInstructionData> protected_instructions_;
 
   DISALLOW_COPY_AND_ASSIGN(LiftoffCompilationUnit);
 };

src/wasm/wasm-code-manager.cc

@@ -131,7 +131,7 @@ void WasmCode::RegisterTrapHandlerData() {
   size_t size = instructions().size();
   const int index =
       RegisterHandlerData(base, size, protected_instructions().size(),
-                          protected_instructions().data());
+                          protected_instructions().start());
 
   // TODO(eholk): if index is negative, fail.
   CHECK_LE(0, index);
@@ -365,7 +365,7 @@ WasmCode* NativeModule::AddOwnedCode(
     Maybe<uint32_t> index, WasmCode::Kind kind, size_t constant_pool_offset,
     uint32_t stack_slots, size_t safepoint_table_offset,
     size_t handler_table_offset,
-    std::unique_ptr<ProtectedInstructions> protected_instructions,
+    OwnedVector<trap_handler::ProtectedInstructionData> protected_instructions,
     WasmCode::Tier tier, WasmCode::FlushICache flush_icache) {
   // both allocation and insertion in owned_code_ happen in the same critical
   // section, thus ensuring owned_code_'s elements are rarely if ever moved.
@@ -464,8 +464,6 @@ WasmCode* NativeModule::AddAnonymousCode(Handle<Code> code,
     source_pos.reset(new byte[source_pos_table->length()]);
     source_pos_table->copy_out(0, source_pos.get(), source_pos_table->length());
   }
-  std::unique_ptr<ProtectedInstructions> protected_instructions(
-      new ProtectedInstructions(0));
   Vector<const byte> orig_instructions(
       reinterpret_cast<byte*>(code->InstructionStart()),
       static_cast<size_t>(code->InstructionSize()));
@@ -478,15 +476,15 @@ WasmCode* NativeModule::AddAnonymousCode(Handle<Code> code,
                    static_cast<size_t>(code->relocation_size()),  // reloc_size
                    std::move(source_pos),  // source positions
                    static_cast<size_t>(source_pos_table->length()),
-                   Nothing<uint32_t>(),                // index
-                   kind,                               // kind
-                   code->constant_pool_offset(),       // constant_pool_offset
-                   stack_slots,                        // stack_slots
-                   safepoint_table_offset,             // safepoint_table_offset
-                   code->handler_table_offset(),       // handler_table_offset
-                   std::move(protected_instructions),  // protected_instructions
-                   WasmCode::kOther,                   // kind
-                   WasmCode::kNoFlushICache);          // flush_icache
+                   Nothing<uint32_t>(),           // index
+                   kind,                          // kind
+                   code->constant_pool_offset(),  // constant_pool_offset
+                   stack_slots,                   // stack_slots
+                   safepoint_table_offset,        // safepoint_table_offset
+                   code->handler_table_offset(),  // handler_table_offset
+                   {},                            // protected_instructions
+                   WasmCode::kOther,              // kind
+                   WasmCode::kNoFlushICache);     // flush_icache
 
   // Apply the relocation delta by iterating over the RelocInfo.
   intptr_t delta = ret->instruction_start() - code->InstructionStart();
@@ -521,7 +519,7 @@ WasmCode* NativeModule::AddAnonymousCode(Handle<Code> code,
 WasmCode* NativeModule::AddCode(
     const CodeDesc& desc, uint32_t frame_slots, uint32_t index,
     size_t safepoint_table_offset, size_t handler_table_offset,
-    std::unique_ptr<ProtectedInstructions> protected_instructions,
+    OwnedVector<trap_handler::ProtectedInstructionData> protected_instructions,
     OwnedVector<byte> source_pos_table, WasmCode::Tier tier) {
   std::unique_ptr<byte[]> reloc_info;
   if (desc.reloc_size) {

src/wasm/wasm-code-manager.h

@@ -86,9 +86,6 @@ class V8_EXPORT_PRIVATE DisjointAllocationPool final {
   DISALLOW_COPY_AND_ASSIGN(DisjointAllocationPool)
 };
 
-using ProtectedInstructions =
-    std::vector<trap_handler::ProtectedInstructionData>;
-
 class V8_EXPORT_PRIVATE WasmCode final {
  public:
   enum Kind {
@@ -144,11 +141,9 @@ class V8_EXPORT_PRIVATE WasmCode final {
            pc < reinterpret_cast<Address>(instructions_.end());
   }
 
-  const ProtectedInstructions& protected_instructions() const {
-    // TODO(mstarzinger): Code that doesn't have trapping instruction should
-    // not be required to have this vector, make it possible to be null.
-    DCHECK_NOT_NULL(protected_instructions_);
-    return *protected_instructions_.get();
+  Vector<trap_handler::ProtectedInstructionData> protected_instructions()
+      const {
+    return protected_instructions_.as_vector();
   }
 
   void Validate() const;
@@ -177,7 +172,8 @@ class V8_EXPORT_PRIVATE WasmCode final {
            Maybe<uint32_t> index, Kind kind, size_t constant_pool_offset,
            uint32_t stack_slots, size_t safepoint_table_offset,
            size_t handler_table_offset,
-           std::unique_ptr<ProtectedInstructions> protected_instructions,
+           OwnedVector<trap_handler::ProtectedInstructionData>
+               protected_instructions,
            Tier tier)
       : instructions_(instructions),
         reloc_info_(std::move(reloc_info)),
@@ -225,7 +221,7 @@ class V8_EXPORT_PRIVATE WasmCode final {
   size_t safepoint_table_offset_ = 0;
   size_t handler_table_offset_ = 0;
   intptr_t trap_handler_index_ = -1;
-  std::unique_ptr<ProtectedInstructions> protected_instructions_;
+  OwnedVector<trap_handler::ProtectedInstructionData> protected_instructions_;
   Tier tier_;
 
   DISALLOW_COPY_AND_ASSIGN(WasmCode);
@@ -238,7 +234,8 @@ class V8_EXPORT_PRIVATE NativeModule final {
  public:
   WasmCode* AddCode(const CodeDesc& desc, uint32_t frame_count, uint32_t index,
                     size_t safepoint_table_offset, size_t handler_table_offset,
-                    std::unique_ptr<ProtectedInstructions>,
+                    OwnedVector<trap_handler::ProtectedInstructionData>
+                        protected_instructions,
                     OwnedVector<byte> source_position_table,
                     WasmCode::Tier tier);
 
@@ -361,8 +358,8 @@ class V8_EXPORT_PRIVATE NativeModule final {
                          WasmCode::Kind kind, size_t constant_pool_offset,
                          uint32_t stack_slots, size_t safepoint_table_offset,
                          size_t handler_table_offset,
-                         std::unique_ptr<ProtectedInstructions>, WasmCode::Tier,
-                         WasmCode::FlushICache);
+                         OwnedVector<trap_handler::ProtectedInstructionData>,
+                         WasmCode::Tier, WasmCode::FlushICache);
 
   WasmCode* CreateEmptyJumpTable(uint32_t num_wasm_functions);
 

src/wasm/wasm-serialization.cc

@@ -301,10 +301,7 @@ void NativeModuleSerializer::WriteCode(const WasmCode* code, Writer* writer) {
   // Write the reloc info, source positions, and protected code.
   writer->WriteVector(code->reloc_info());
   writer->WriteVector(code->source_positions());
-  writer->WriteVector(
-      {reinterpret_cast<const byte*>(code->protected_instructions().data()),
-       sizeof(trap_handler::ProtectedInstructionData) *
-           code->protected_instructions().size()});
+  writer->WriteVector(Vector<byte>::cast(code->protected_instructions()));
 #if V8_TARGET_ARCH_MIPS || V8_TARGET_ARCH_MIPS64 || V8_TARGET_ARCH_ARM
   // On platforms that don't support misaligned word stores, copy to an aligned
   // buffer if necessary so we can relocate the serialized code.
@@ -458,15 +455,10 @@ bool NativeModuleDeserializer::ReadCode(uint32_t fn_index, Reader* reader) {
     source_pos.reset(new byte[source_position_size]);
     reader->ReadVector({source_pos.get(), source_position_size});
   }
-  std::unique_ptr<ProtectedInstructions> protected_instructions(
-      new ProtectedInstructions(protected_instructions_size));
-  if (protected_instructions_size > 0) {
-    size_t size = sizeof(trap_handler::ProtectedInstructionData) *
-                  protected_instructions->size();
-    Vector<byte> data(reinterpret_cast<byte*>(protected_instructions->data()),
-                      size);
-    reader->ReadVector(data);
-  }
+  auto protected_instructions =
+      OwnedVector<trap_handler::ProtectedInstructionData>::New(
+          protected_instructions_size);
+  reader->ReadVector(Vector<byte>::cast(protected_instructions.as_vector()));
   WasmCode* ret = native_module_->AddOwnedCode(
       code_buffer, std::move(reloc_info), reloc_size, std::move(source_pos),
       source_position_size, Just(fn_index), WasmCode::kFunction,

test/cctest/wasm/test-run-wasm.cc

@@ -3438,10 +3438,7 @@ TEST(Liftoff_tier_up) {
     memcpy(buffer.get(), sub_code->instructions().start(), sub_size);
     desc.buffer = buffer.get();
     desc.instr_size = static_cast<int>(sub_size);
-    std::unique_ptr<ProtectedInstructions> protected_instructions(
-        new ProtectedInstructions(sub_code->protected_instructions()));
-    native_module->AddCode(desc, 0, add.function_index(), 0, 0,
-                           std::move(protected_instructions),
+    native_module->AddCode(desc, 0, add.function_index(), 0, 0, {},
                            OwnedVector<byte>(), WasmCode::kOther);
 
     // Second run should now execute {sub}.