More cleanup to avoid creation of redundant handles

runtime/vm/object.h

Index: runtime/vm/object.h
===================================================================
--- runtime/vm/object.h	(revision 23418)
+++ runtime/vm/object.h	(working copy)
@@ -110,9 +110,16 @@
     ASSERT(obj.Is##object());                                                  \
     return reinterpret_cast<const object&>(obj);                               \
   }                                                                            \
+  static Raw##object* RawCast(RawObject* raw) {                                \
+    ASSERT(Object::Handle(raw).Is##object());                                  \

[hausner, 2013/05/31 16:40:25]

I always cringe at expensive asserts in very low level functions. The difference
between debug and release code in terms of performance should be reasonably
small, otherwise debug builds become unusable.

I would let the call site of this function determine whether an assertion is
desired or not.

In fact, I think the assertion even introduces a bug. Allocating a handle can
cause the GC to run and invalidate the raw pointer that is passed to RawCast.

[siva, 2013/05/31 22:35:48]

A handle creation can not lead to garbage collection as it is a C
heap allocation. It can lead to an OOM from the process which is
possible anywhere.

Regarding the assert making the debug build slower that is true.
We have a number of such checks which are true in debug mode.
I need to see how this check can be made less expensive in
a new CL.

On 2013/05/31 16:40:25, hausner wrote:

+    return reinterpret_cast<Raw##object*>(raw);                                \
+  }                                                                            \
   static Raw##object* null() {                                                 \
     return reinterpret_cast<Raw##object*>(Object::null());                     \
   }                                                                            \
+  static const object& null_object() {                                         \
+    return reinterpret_cast<const object&>(Object::null_object());             \
+  }                                                                            \
   virtual const char* ToCString() const;                                       \
   static const ClassId kClassId = k##object##Cid;                              \
  private:  /* NOLINT */                                                        \
@@ -305,6 +312,10 @@
   }
 
   static RawObject* null() { return null_; }
+  static const Object& null_object() {
+    ASSERT(null_object_ != NULL);
+    return *null_object_;
+  }
   static const Array& empty_array() {
     ASSERT(empty_array_ != NULL);
     return *empty_array_;
@@ -342,6 +353,11 @@
     ASSERT(bool_false_ != NULL);
     return *bool_false_;
   }
+
+  static const Smi& sentinel_smi() {
+    ASSERT(sentinel_smi_ != NULL);
+    return *sentinel_smi_;
+  }
   static const LanguageError& snapshot_writer_error() {
     ASSERT(snapshot_writer_error_ != NULL);
     return *snapshot_writer_error_;
@@ -533,6 +549,7 @@
 
   // The static values below are read-only handle pointers for singleton
   // objects that are shared between the different isolates.
+  static Object* null_object_;
   static Array* empty_array_;
   static Instance* sentinel_;
   static Instance* transition_sentinel_;
@@ -540,6 +557,7 @@
   static Instance* non_constant_;
   static Bool* bool_true_;
   static Bool* bool_false_;
+  static Smi* sentinel_smi_;

[hausner, 2013/05/31 16:40:25]

I would name this Smi illegal_cid or something similar, to make the connection
with the value it holds (kIllegalCid).

[siva, 2013/05/31 22:35:48]

Renamed it smi_illegal_cid_

On 2013/05/31 16:40:25, hausner wrote:

   static LanguageError* snapshot_writer_error_;
 
   friend void ClassTable::Register(const Class& cls);
@@ -3251,7 +3269,7 @@
 #endif  // DEBUG
 
   intptr_t TestEntryLength() const;
-  void WriteSentinel() const;
+  void WriteSentinel(const Array& data) const;
 
   FINAL_HEAP_OBJECT_IMPLEMENTATION(ICData, Object);
   friend class Class;