Encode interpreter::SourcePositionTable as variable-length ints.

src/interpreter/source-position-table.h

Index: src/interpreter/source-position-table.h
diff --git a/src/interpreter/source-position-table.h b/src/interpreter/source-position-table.h
index 336cf42bc2c1fe00e4205d5f9372539a37a02b7e..8599b22b4b70645ce8b38667fe346c46aa50ca3f 100644
--- a/src/interpreter/source-position-table.h
+++ b/src/interpreter/source-position-table.h
@@ -6,43 +6,68 @@
 #define V8_INTERPRETER_SOURCE_POSITION_TABLE_H_
 
 #include "src/assert-scope.h"
+#include "src/checks.h"
 #include "src/handles.h"
-#include "src/zone.h"
 #include "src/zone-containers.h"
 
 namespace v8 {
 namespace internal {
 
 class BytecodeArray;
-class FixedArray;
+class ByteArray;
 class Isolate;
+class Zone;
 
 namespace interpreter {
 
+struct PositionTableEntry {
+  int bytecode_offset;
+  int source_position;
+  bool is_statement;
+};
+
+// Helper class for SourcePositionTableBuilder + SourcePositionTableIterator.
+// Each instance is meant for either encoding or decoding, but not both.
+class SourcePositionTableCodec {

[Yang, 2016/02/17 19:54:51]

I expect this to be only used by the builder and the iterator. Can we put this
class in an anonymous namespace?

[vogelheim, 2016/02/18 13:06:09]

I removed it entirely, per your other suggestion.

+ public:
+  SourcePositionTableCodec();
+  ~SourcePositionTableCodec();
+
+  void Encode(ZoneVector<byte>& bytes, PositionTableEntry entry);
+  void EncodeRevertPosition(ZoneVector<byte>& bytes, int bytecode_offset);
+  void Decode(const byte* bytes, int* index, PositionTableEntry* entry);
+  int GetPreviousBytecodeOffset() const { return previous_bytecode_offset_; }
+
+ private:
+  int previous_bytecode_offset_;
+  int previous_source_position_;
+};
+
 class SourcePositionTableBuilder {
  public:
   explicit SourcePositionTableBuilder(Isolate* isolate, Zone* zone)
-      : isolate_(isolate), entries_(zone) {}
+      : isolate_(isolate), bytes_(zone) {}
 
   void AddStatementPosition(size_t bytecode_offset, int source_position);
   void AddExpressionPosition(size_t bytecode_offset, int source_position);
   void RevertPosition(size_t bytecode_offset);
-  Handle<FixedArray> ToFixedArray();
+  Handle<ByteArray> ToSourcePositionTable();
 
  private:
-  struct Entry {
-    int bytecode_offset;
-    uint32_t source_position_and_type;
-  };
-
+  void AddEntry(const PositionTableEntry& entry);
   bool CodeOffsetHasPosition(int bytecode_offset) {
     // Return whether bytecode offset already has a position assigned.
-    return entries_.size() > 0 &&
-           entries_.back().bytecode_offset == bytecode_offset;
+    return bytes_.size() > 0 &&
+           codec_.GetPreviousBytecodeOffset() == bytecode_offset;
   }
 
   Isolate* isolate_;
-  ZoneVector<Entry> entries_;
+  ZoneVector<byte> bytes_;
+  SourcePositionTableCodec codec_;
+
+#ifdef ENABLE_SLOW_DCHECKS
+  std::vector<PositionTableEntry> raw_entries_;
+#endif
 };
 
 class SourcePositionTableIterator {
@@ -53,25 +78,29 @@ class SourcePositionTableIterator {
 
   int bytecode_offset() const {
     DCHECK(!done());
-    return bytecode_offset_;
+    return current_.bytecode_offset;
   }
   int source_position() const {
     DCHECK(!done());
-    return source_position_;
+    return current_.source_position;
   }
   bool is_statement() const {
     DCHECK(!done());
-    return is_statement_;
+    return current_.is_statement;
   }
-  bool done() const { return index_ > length_; }
+  bool done() const { return index_ == kDone; }
 
  private:
-  FixedArray* table_;
+  static const int kDone = -1;
+
+  // Allow access to private constructor to implement the testing functionality.
+  friend Handle<ByteArray> SourcePositionTableBuilder::ToSourcePositionTable();
+  explicit SourcePositionTableIterator(ByteArray* byte_array);

[Yang, 2016/02/17 19:54:51]

I'd actually be perfectly fine if we changed the public constructor to take a
ByteArray* and change all the call sites. Your decision :)

[vogelheim, 2016/02/18 13:06:09]

Done. I like your version better.

+
+  ByteArray* table_;
   int index_;
-  int length_;
-  bool is_statement_;
-  int bytecode_offset_;
-  int source_position_;
+  SourcePositionTableCodec codec_;
+  PositionTableEntry current_;
   DisallowHeapAllocation no_gc;
 };