[vm] Add (S)LEB128 encoding/decoding to BaseWriteStream.

Unlike cfc8e6d, this does _not_ replace the default variable length
encoding for {Read,Write}Streams, but insteads adds separate
{Read,Write}{S,}LEB128 methods to the appropriate classes. If we later
find the cause of the issues that led to the revert of cfc8e6d, it'll
be easy to switch over then.

Note that WriteLEB128 asserts that the value is non-negative if used
with a signed type (since negative values suggests that SLEB128 should
be used instead for minimal encoding).

Also removes the various other encoding and decoding methods for
(S)LEB128 across the codebase and changes those clients to use
{Read,Write}Streams instead.

Other cleanups:

* Various constant-related cleanups in datastream.h.

* Adds DART_FORCE_INLINE to ReadStream::ReadByte and uses it in the
  default variable length decoding methods for retrieving bytes
  from the stream instead of managing current_ by hand.

* Creates a canonical empty CompressedStackMaps instance and uses
  that instead of the null CompressedStackMaps instance in most cases.
  The only remaining (expected) use of the null CompressedStackMaps
  instance is for the global table in the object store when no global
  table exists (e.g., in JIT mode before any snapshotting).

* Moves CompressedStackMapsIterator from code_descriptors.h to an
  Iterator class within CompressedStackMaps in object.h (similar to
  PcDescriptors::Iterator), to limit friend declarations and because it
  conceptually makes more sense as part of CompressedStackMaps.

* Removed CompressedStackMaps::PayloadByte, since existing clients
  (CompressedStackMaps::Iterator, StackMapEntry in program_visitor.cc)
  are better served by just operating on the payload buffer directly
  (with appropriate NoSafepointScopes).

* WriteStreams no longer allocate their initial space on construction,
  but rather on the first write, so no allocation is performed by
  constructing a never-used WriteStream.

Cq-Include-Trybots: luci.dart.try:vm-kernel-precomp-linux-debug-x64-try,vm-kernel-precomp-linux-debug-simarm_x64-try,vm-kernel-precomp-mac-release-simarm64-try,vm-kernel-mac-debug-x64-try,vm-kernel-win-debug-x64-try,vm-kernel-win-debug-ia32-try,vm-kernel-precomp-win-release-x64-try,vm-kernel-ubsan-linux-release-x64-try,vm-kernel-tsan-linux-release-x64-try,vm-kernel-precomp-ubsan-linux-release-x64-try,vm-kernel-precomp-tsan-linux-release-x64-try,vm-kernel-precomp-msan-linux-release-x64-try,vm-kernel-precomp-asan-linux-release-x64-try,vm-kernel-msan-linux-release-x64-try,vm-kernel-asan-linux-release-x64-try
Change-Id: Ice63321abaa79157fbe9f230a864c8bba0e6dea9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/166421
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>

Author: sstrickl

runtime/platform/utils.h

@@ -387,29 +387,6 @@ class Utils {
     return ((mask >> position) & 1) != 0;
   }
 
-  // Decode integer in SLEB128 format from |data| and update |byte_index|.
-  template <typename ValueType>
-  static ValueType DecodeSLEB128(const uint8_t* data,
-                                 const intptr_t data_length,
-                                 intptr_t* byte_index) {
-    using Unsigned = typename std::make_unsigned<ValueType>::type;
-    ASSERT(*byte_index < data_length);
-    uword shift = 0;
-    Unsigned value = 0;
-    uint8_t part = 0;
-    do {
-      part = data[(*byte_index)++];
-      value |= static_cast<Unsigned>(part & 0x7f) << shift;
-      shift += 7;
-    } while ((part & 0x80) != 0);
-
-    if ((shift < (sizeof(ValueType) * CHAR_BIT)) && ((part & 0x40) != 0)) {
-      const Unsigned kMax = std::numeric_limits<Unsigned>::max();
-      value |= static_cast<Unsigned>(kMax << shift);
-    }
-    return static_cast<ValueType>(value);
-  }
-
   static char* StrError(int err, char* buffer, size_t bufsize);
 
   // Not all platforms support strndup.

runtime/vm/bitmap.cc

@@ -77,7 +77,7 @@ void BitmapBuilder::Print() const {
   }
 }
 
-void BitmapBuilder::AppendAsBytesTo(GrowableArray<uint8_t>* bytes) const {
+void BitmapBuilder::AppendAsBytesTo(BaseWriteStream* stream) const {
   // Early return if there are no bits in the payload to copy.
   if (Length() == 0) return;
 
@@ -94,19 +94,20 @@ void BitmapBuilder::AppendAsBytesTo(GrowableArray<uint8_t>* bytes) const {
     payload_size = total_size;
     extra_size = 0;
   }
+#if defined(DEBUG)
+  // Make sure any bits in the payload beyond the bit length if we're not
+  // appending trailing zeroes are cleared to ensure deterministic snapshots.
+  if (extra_size == 0 && Length() % kBitsPerByte != 0) {
+    const int8_t mask = (1 << (Length() % kBitsPerByte)) - 1;
+    ASSERT_EQUAL(data_[payload_size - 1], (data_[payload_size - 1] & mask));
+  }
+#endif
   for (intptr_t i = 0; i < payload_size; i++) {
-    bytes->Add(data_[i]);
+    stream->WriteByte(data_[i]);
   }
   for (intptr_t i = 0; i < extra_size; i++) {
-    bytes->Add(0U);
+    stream->WriteByte(0U);
   }
-  // Make sure any bits in the payload beyond the bit length are cleared to
-  // ensure deterministic snapshots.
-#if defined(DEBUG)
-  if (Length() % kBitsPerByte == 0) return;
-  const int8_t mask = (1 << (Length() % kBitsPerByte)) - 1;
-  ASSERT(bytes->Last() == (bytes->Last() & mask));
-#endif
 }
 
 bool BitmapBuilder::GetBit(intptr_t bit_offset) const {

runtime/vm/bitmap.h

@@ -6,7 +6,7 @@
 #define RUNTIME_VM_BITMAP_H_
 
 #include "vm/allocation.h"
-#include "vm/growable_array.h"
+#include "vm/datastream.h"
 #include "vm/thread_state.h"
 #include "vm/zone.h"
 
@@ -44,7 +44,7 @@ class BitmapBuilder : public ZoneAllocated {
   void SetRange(intptr_t min, intptr_t max, bool value);
 
   void Print() const;
-  void AppendAsBytesTo(GrowableArray<uint8_t>* bytes) const;
+  void AppendAsBytesTo(BaseWriteStream* stream) const;
 
  private:
   static const intptr_t kInitialSizeInBytes = 16;

runtime/vm/bitmap_test.cc

@@ -16,8 +16,8 @@ namespace dart {
 static const uint32_t kTestPcOffset = 0x4;
 static const intptr_t kTestSpillSlotBitCount = 0;
 
-static CompressedStackMapsPtr MapsFromBuilder(BitmapBuilder* bmap) {
-  CompressedStackMapsBuilder builder;
+static CompressedStackMapsPtr MapsFromBuilder(Zone* zone, BitmapBuilder* bmap) {
+  CompressedStackMapsBuilder builder(zone);
   builder.AddEntry(kTestPcOffset, bmap, kTestSpillSlotBitCount);
   return builder.Finalize();
 }
@@ -51,8 +51,9 @@ ISOLATE_UNIT_TEST_CASE(BitmapBuilder) {
   }
 
   // Create a CompressedStackMaps object and verify its contents.
-  const auto& maps1 = CompressedStackMaps::Handle(MapsFromBuilder(builder1));
-  CompressedStackMapsIterator it1(maps1);
+  const auto& maps1 = CompressedStackMaps::Handle(
+      thread->zone(), MapsFromBuilder(thread->zone(), builder1));
+  CompressedStackMaps::Iterator it1(thread, maps1);
   EXPECT(it1.MoveNext());
 
   EXPECT_EQ(kTestPcOffset, it1.pc_offset());
@@ -83,8 +84,9 @@ ISOLATE_UNIT_TEST_CASE(BitmapBuilder) {
     EXPECT(!builder1->Get(i));
   }
 
-  const auto& maps2 = CompressedStackMaps::Handle(MapsFromBuilder(builder1));
-  CompressedStackMapsIterator it2(maps2);
+  const auto& maps2 = CompressedStackMaps::Handle(
+      thread->zone(), MapsFromBuilder(thread->zone(), builder1));
+  CompressedStackMaps::Iterator it2(thread, maps2);
   EXPECT(it2.MoveNext());
 
   EXPECT_EQ(kTestPcOffset, it2.pc_offset());

runtime/vm/code_descriptors.cc

@@ -35,37 +35,25 @@ void DescriptorList::AddDescriptor(PcDescriptorsLayout::Kind kind,
         PcDescriptorsLayout::KindAndMetadata::Encode(kind, try_index,
                                                      yield_index);
 
-    PcDescriptors::EncodeInteger(&encoded_data_, kind_and_metadata);
-    PcDescriptors::EncodeInteger(&encoded_data_, pc_offset - prev_pc_offset);
+    encoded_data_.WriteSLEB128(kind_and_metadata);
+    encoded_data_.WriteSLEB128(pc_offset - prev_pc_offset);
     prev_pc_offset = pc_offset;
 
     if (!FLAG_precompiled_mode) {
-      PcDescriptors::EncodeInteger(&encoded_data_, deopt_id - prev_deopt_id);
-      PcDescriptors::EncodeInteger(&encoded_data_,
-                                   token_pos.value() - prev_token_pos);
+      encoded_data_.WriteSLEB128(deopt_id - prev_deopt_id);
+      encoded_data_.WriteSLEB128(token_pos.value() - prev_token_pos);
       prev_deopt_id = deopt_id;
       prev_token_pos = token_pos.value();
     }
   }
 }
 
 PcDescriptorsPtr DescriptorList::FinalizePcDescriptors(uword entry_point) {
-  if (encoded_data_.length() == 0) {
+  if (encoded_data_.bytes_written() == 0) {
     return Object::empty_descriptors().raw();
   }
-  return PcDescriptors::New(&encoded_data_);
-}
-
-// Encode unsigned integer |value| in LEB128 format and store into |data|.
-void CompressedStackMapsBuilder::EncodeLEB128(GrowableArray<uint8_t>* data,
-                                              uintptr_t value) {
-  while (true) {
-    uint8_t part = value & 0x7f;
-    value >>= 7;
-    if (value != 0) part |= 0x80;
-    data->Add(part);
-    if (value == 0) break;
-  }
+  return PcDescriptors::New(encoded_data_.buffer(),
+                            encoded_data_.bytes_written());
 }
 
 void CompressedStackMapsBuilder::AddEntry(intptr_t pc_offset,
@@ -74,179 +62,22 @@ void CompressedStackMapsBuilder::AddEntry(intptr_t pc_offset,
   ASSERT(bitmap != nullptr);
   ASSERT(pc_offset > last_pc_offset_);
   ASSERT(spill_slot_bit_count >= 0 && spill_slot_bit_count <= bitmap->Length());
-  auto const pc_delta = pc_offset - last_pc_offset_;
-  auto const non_spill_slot_bit_count = bitmap->Length() - spill_slot_bit_count;
-  EncodeLEB128(&encoded_bytes_, pc_delta);
-  EncodeLEB128(&encoded_bytes_, spill_slot_bit_count);
-  EncodeLEB128(&encoded_bytes_, non_spill_slot_bit_count);
+  const uword pc_delta = pc_offset - last_pc_offset_;
+  const uword non_spill_slot_bit_count =
+      bitmap->Length() - spill_slot_bit_count;
+  encoded_bytes_.WriteLEB128(pc_delta);
+  encoded_bytes_.WriteLEB128(spill_slot_bit_count);
+  encoded_bytes_.WriteLEB128(non_spill_slot_bit_count);
   bitmap->AppendAsBytesTo(&encoded_bytes_);
   last_pc_offset_ = pc_offset;
 }
 
 CompressedStackMapsPtr CompressedStackMapsBuilder::Finalize() const {
-  if (encoded_bytes_.length() == 0) return CompressedStackMaps::null();
-  return CompressedStackMaps::NewInlined(encoded_bytes_);
-}
-
-CompressedStackMapsIterator::CompressedStackMapsIterator(
-    const CompressedStackMaps& maps,
-    const CompressedStackMaps& global_table)
-    : maps_(maps),
-      bits_container_(maps_.UsesGlobalTable() ? global_table : maps_) {
-  ASSERT(!maps_.IsGlobalTable());
-  ASSERT(!maps_.UsesGlobalTable() || bits_container_.IsGlobalTable());
-}
-
-CompressedStackMapsIterator::CompressedStackMapsIterator(
-    const CompressedStackMaps& maps)
-    : CompressedStackMapsIterator(
-          maps,
-          // Only look up the global table if the map will end up using it.
-          maps.UsesGlobalTable() ? CompressedStackMaps::Handle(
-                                       Thread::Current()
-                                           ->isolate()
-                                           ->object_store()
-                                           ->canonicalized_stack_map_entries())
-                                 : Object::null_compressed_stack_maps()) {}
-
-CompressedStackMapsIterator::CompressedStackMapsIterator(
-    const CompressedStackMapsIterator& it)
-    : maps_(it.maps_),
-      bits_container_(it.bits_container_),
-      next_offset_(it.next_offset_),
-      current_pc_offset_(it.current_pc_offset_),
-      current_global_table_offset_(it.current_global_table_offset_),
-      current_spill_slot_bit_count_(it.current_spill_slot_bit_count_),
-      current_non_spill_slot_bit_count_(it.current_spill_slot_bit_count_),
-      current_bits_offset_(it.current_bits_offset_) {}
-
-// Decode unsigned integer in LEB128 format from the payload of |maps| and
-// update |byte_index|.
-uintptr_t CompressedStackMapsIterator::DecodeLEB128(
-    const CompressedStackMaps& maps,
-    uintptr_t* byte_index) {
-  uword shift = 0;
-  uintptr_t value = 0;
-  uint8_t part = 0;
-  do {
-    ASSERT(*byte_index < maps.payload_size());
-    part = maps.PayloadByte((*byte_index)++);
-    value |= static_cast<uintptr_t>(part & 0x7f) << shift;
-    shift += 7;
-  } while ((part & 0x80) != 0);
-
-  return value;
-}
-
-bool CompressedStackMapsIterator::MoveNext() {
-  // Empty CompressedStackMaps are represented as null values.
-  if (maps_.IsNull() || next_offset_ >= maps_.payload_size()) return false;
-  uintptr_t offset = next_offset_;
-
-  auto const pc_delta = DecodeLEB128(maps_, &offset);
-  ASSERT(pc_delta <= (kMaxUint32 - current_pc_offset_));
-  current_pc_offset_ += pc_delta;
-
-  // Table-using CSMs have a table offset after the PC offset delta, whereas
-  // the post-delta part of inlined entries has the same information as
-  // global table entries.
-  if (maps_.UsesGlobalTable()) {
-    current_global_table_offset_ = DecodeLEB128(maps_, &offset);
-    ASSERT(current_global_table_offset_ < bits_container_.payload_size());
-
-    // Since generally we only use entries in the GC and the GC only needs
-    // the rest of the entry information if the PC offset matches, we lazily
-    // load and cache the information stored in the global object when it is
-    // actually requested.
-    current_spill_slot_bit_count_ = -1;
-    current_non_spill_slot_bit_count_ = -1;
-    current_bits_offset_ = -1;
-  } else {
-    current_spill_slot_bit_count_ = DecodeLEB128(maps_, &offset);
-    ASSERT(current_spill_slot_bit_count_ >= 0);
-
-    current_non_spill_slot_bit_count_ = DecodeLEB128(maps_, &offset);
-    ASSERT(current_non_spill_slot_bit_count_ >= 0);
-
-    const auto stackmap_bits =
-        current_spill_slot_bit_count_ + current_non_spill_slot_bit_count_;
-    const uintptr_t stackmap_size =
-        Utils::RoundUp(stackmap_bits, kBitsPerByte) >> kBitsPerByteLog2;
-    ASSERT(stackmap_size <= (maps_.payload_size() - offset));
-
-    current_bits_offset_ = offset;
-    offset += stackmap_size;
-  }
-
-  next_offset_ = offset;
-  return true;
-}
-
-intptr_t CompressedStackMapsIterator::Length() {
-  EnsureFullyLoadedEntry();
-  return current_spill_slot_bit_count_ + current_non_spill_slot_bit_count_;
-}
-intptr_t CompressedStackMapsIterator::SpillSlotBitCount() {
-  EnsureFullyLoadedEntry();
-  return current_spill_slot_bit_count_;
-}
-
-bool CompressedStackMapsIterator::IsObject(intptr_t bit_index) {
-  EnsureFullyLoadedEntry();
-  ASSERT(!bits_container_.IsNull());
-  ASSERT(bit_index >= 0 && bit_index < Length());
-  const intptr_t byte_index = bit_index >> kBitsPerByteLog2;
-  const intptr_t bit_remainder = bit_index & (kBitsPerByte - 1);
-  uint8_t byte_mask = 1U << bit_remainder;
-  const intptr_t byte_offset = current_bits_offset_ + byte_index;
-  return (bits_container_.PayloadByte(byte_offset) & byte_mask) != 0;
-}
-
-void CompressedStackMapsIterator::LazyLoadGlobalTableEntry() {
-  ASSERT(maps_.UsesGlobalTable() && bits_container_.IsGlobalTable());
-  ASSERT(HasLoadedEntry());
-  ASSERT(current_global_table_offset_ < bits_container_.payload_size());
-
-  uintptr_t offset = current_global_table_offset_;
-  current_spill_slot_bit_count_ = DecodeLEB128(bits_container_, &offset);
-  ASSERT(current_spill_slot_bit_count_ >= 0);
-
-  current_non_spill_slot_bit_count_ = DecodeLEB128(bits_container_, &offset);
-  ASSERT(current_non_spill_slot_bit_count_ >= 0);
-
-  const auto stackmap_bits = Length();
-  const uintptr_t stackmap_size =
-      Utils::RoundUp(stackmap_bits, kBitsPerByte) >> kBitsPerByteLog2;
-  ASSERT(stackmap_size <= (bits_container_.payload_size() - offset));
-
-  current_bits_offset_ = offset;
-}
-
-const char* CompressedStackMapsIterator::ToCString(Zone* zone) const {
-  ZoneTextBuffer b(zone, 100);
-  CompressedStackMapsIterator it(*this);
-  // If we haven't loaded an entry yet, do so (but don't skip the current
-  // one if we have!)
-  if (!it.HasLoadedEntry()) {
-    if (!it.MoveNext()) return b.buffer();
-  }
-  bool first_entry = true;
-  do {
-    if (first_entry) {
-      first_entry = false;
-    } else {
-      b.AddString("\n");
-    }
-    b.Printf("0x%08x: ", it.pc_offset());
-    for (intptr_t i = 0, n = it.Length(); i < n; i++) {
-      b.AddString(it.IsObject(i) ? "1" : "0");
-    }
-  } while (it.MoveNext());
-  return b.buffer();
-}
-
-const char* CompressedStackMapsIterator::ToCString() const {
-  return ToCString(Thread::Current()->zone());
+  if (encoded_bytes_.bytes_written() == 0) {
+    return Object::empty_compressed_stackmaps().raw();
+  }
+  return CompressedStackMaps::NewInlined(encoded_bytes_.buffer(),
+                                         encoded_bytes_.bytes_written());
 }
 
 ExceptionHandlersPtr ExceptionHandlerList::FinalizeExceptionHandlers(