Uniform and precise source positions for inlining

src/objects.h

Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index 9e3c0372ce2e34c091dcda0510015f217c09b823..7d98a919ab2db6278a4c3fabbcaee49513715243 100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -168,6 +168,8 @@
 namespace v8 {
 namespace internal {
 
+struct InliningPosition;
+
 enum KeyedAccessStoreMode {
   STANDARD_STORE,
   STORE_TRANSITION_TO_OBJECT,
@@ -4960,6 +4962,32 @@ class ByteArray: public FixedArrayBase {
   DISALLOW_IMPLICIT_CONSTRUCTORS(ByteArray);
 };
 
+// Wrapper class for ByteArray which can store arbitrary C++ classes, as long
+// as they can be copied with memcpy.
+template <class T>
+class PodArray : public ByteArray {
+ public:
+  static Handle<PodArray<T>> New(Isolate* isolate, int length,
+                                 PretenureFlag pretenure = NOT_TENURED);
+  void copy_out(int index, T* result) {
+    ByteArray::copy_out(index * sizeof(T), reinterpret_cast<byte*>(result),
+                        sizeof(T));
+  }
+  T get(int index) {
+    T result;
+    copy_out(index, &result);
+    return result;
+  }
+  void set(int index, const T& value) {
+    copy_in(index * sizeof(T), reinterpret_cast<const byte*>(&value),
+            sizeof(T));
+  }
+  int length() { return ByteArray::length() / sizeof(T); }
+  DECLARE_CAST(PodArray<T>);
+
+ private:
+  DISALLOW_IMPLICIT_CONSTRUCTORS(PodArray<T>);
+};
 
 // BytecodeArray represents a sequence of interpreter bytecodes.
 class BytecodeArray : public FixedArrayBase {
@@ -5192,7 +5220,6 @@ TYPED_ARRAYS(FIXED_TYPED_ARRAY_TRAITS)
 
 #undef FIXED_TYPED_ARRAY_TRAITS
 
-
 // DeoptimizationInputData is a fixed array used to hold the deoptimization
 // data for code generated by the Hydrogen/Lithium compiler.  It also
 // contains information about functions that were inlined.  If N different
@@ -5211,7 +5238,8 @@ class DeoptimizationInputData: public FixedArray {
   static const int kOptimizationIdIndex = 5;
   static const int kSharedFunctionInfoIndex = 6;
   static const int kWeakCellCacheIndex = 7;
-  static const int kFirstDeoptEntryIndex = 8;
+  static const int kInliningPositionsIndex = 8;
+  static const int kFirstDeoptEntryIndex = 9;
 
   // Offsets of deopt entry elements relative to the start of the entry.
   static const int kAstIdRawOffset = 0;
@@ -5233,6 +5261,7 @@ class DeoptimizationInputData: public FixedArray {
   DECLARE_ELEMENT_ACCESSORS(OptimizationId, Smi)
   DECLARE_ELEMENT_ACCESSORS(SharedFunctionInfo, Object)
   DECLARE_ELEMENT_ACCESSORS(WeakCellCache, Object)
+  DECLARE_ELEMENT_ACCESSORS(InliningPositions, PodArray<InliningPosition>)
 
 #undef DECLARE_ELEMENT_ACCESSORS
 
@@ -5274,7 +5303,6 @@ class DeoptimizationInputData: public FixedArray {
   static int LengthFor(int entry_count) { return IndexForEntry(entry_count); }
 };
 
-
 // DeoptimizationOutputData is a fixed array used to hold the deoptimization
 // data for code generated by the full compiler.
 // The format of the these objects is
@@ -7160,14 +7188,6 @@ class Script: public Struct {
   // Init line_ends array with source code positions of line ends.
   static void InitLineEnds(Handle<Script> script);
 
-  // Convert code offset into column number.
-  static int GetColumnNumber(Handle<Script> script, int code_offset);
-
-  // Convert code offset into (zero-based) line number.
-  // The non-handlified version does not allocate, but may be much slower.
-  static int GetLineNumber(Handle<Script> script, int code_offset);
-  int GetLineNumber(int code_pos);
-
   // Carries information about a source position.
   struct PositionInfo {
     PositionInfo() : line(-1), column(-1), line_start(-1), line_end(-1) {}
@@ -7175,7 +7195,7 @@ class Script: public Struct {
     int line;        // Zero-based line number.
     int column;      // Zero-based column number.
     int line_start;  // Position of first character in line.
-    int line_end;    // Position of last (non-linebreak) character in line.
+    int line_end;    // Position of final linebreak character in line.
   };
 
   // Specifies whether to add offsets to position infos.
@@ -7184,9 +7204,18 @@ class Script: public Struct {
   // Retrieves information about the given position, optionally with an offset.
   // Returns false on failure, and otherwise writes into the given info object
   // on success.
+  // The non-handlified version does not allocate, but may be much slower.
+  static bool GetPositionInfo(Handle<Script> script, int position,
+                              PositionInfo* info, OffsetFlag offset_flag);
   bool GetPositionInfo(int position, PositionInfo* info,
                        OffsetFlag offset_flag);
 
+  // Wrappers for GetPositionInfo
+  static int GetColumnNumber(Handle<Script> script, int code_offset);

[vogelheim, 2016/11/07 17:53:27]

The static instances get GetPositionInfo, GetColumnNumber and GetLineNumber look
like some rather gratuitous code duplication. Is there a reason why we can't
just call script->GetXXXX instead? If something is missing, can we make the
non-static members do that additional work instead?

[Tobias Tebbi, 2016/11/08 10:29:07]

The reason is that the non-static methods are not allocating, but may be slower
because the cannot initialize the line ends array. So handlified callsites
should use the handlified version for performace, while unhandlified callsites
have to resort to the non-static methods. I will clearify the above comment.

+  int GetColumnNumber(int code_pos);

[vogelheim, 2016/11/07 17:53:27]

const method?

[Also GetLineNumber below. And I get GetPositionInfo above..

It looks like making GetPositionInfo const would require ends() to be const,
which requires the underlying member to be mutable. Not sure if any of this is
worth the effort, though. You decide.]

[Tobias Tebbi, 2016/11/08 10:29:07]

Since the underlying member is not even a C++ member, this would require const
casting. Also, the C++11 lib spec requires const methods to be thread-safe
(http://stackoverflow.com/questions/14127379/does-const-mean-thread-safe-in-c11),
so mutable should only be used on locked objects. On the other hand, this update
of a cache should even be a case of lock-free thread safety... And I am not sure
if this applies to V8 at all.

+  static int GetLineNumber(Handle<Script> script, int code_offset);
+  int GetLineNumber(int code_pos);
+
   // Get the JS object wrapping the given script; create it if none exists.
   static Handle<JSObject> GetWrapper(Handle<Script> script);
 
@@ -7229,8 +7258,6 @@ class Script: public Struct {
   static const int kSize = kSourceMappingUrlOffset + kPointerSize;
 
  private:
-  int GetLineNumberWithArray(int code_pos);
-
   // Bit positions in the flags field.
   static const int kCompilationTypeBit = 0;
   static const int kCompilationStateBit = 1;