Add Service ID zones to service protocol

runtime/vm/object_id_ring.cc

Index: runtime/vm/object_id_ring.cc
diff --git a/runtime/vm/object_id_ring.cc b/runtime/vm/object_id_ring.cc
index 0b396e83cae8c5a49953dbfd048cb46626e9087d..13e5cd02ddeba1d44259bfbd04f2b1f941befc16 100644
--- a/runtime/vm/object_id_ring.cc
+++ b/runtime/vm/object_id_ring.cc
@@ -4,6 +4,7 @@
 
 #include "platform/assert.h"
 #include "vm/dart_api_state.h"
+#include "vm/json_stream.h"
 #include "vm/object_id_ring.h"
 
 namespace dart {
@@ -25,14 +26,34 @@ ObjectIdRing::~ObjectIdRing() {
 }
 
 
-int32_t ObjectIdRing::GetIdForObject(RawObject* object) {
+int32_t ObjectIdRing::GetIdForObject(RawObject* object, IdPolicy policy) {
   // We do not allow inserting null because null is how we detect as entry was
   // reclaimed by the GC.
   ASSERT(object != Object::null());
+  if (policy == kNewId) {
+    return AllocateNewId(object);
+  }
+  ASSERT(policy == kExistingOrNewId);
+  int32_t id = FindExistingIdForObject(object);
+  if (id != kInvalidId) {
+    // Return a previous id for |object|.
+    return id;
+  }
   return AllocateNewId(object);
 }
 
 
+int32_t ObjectIdRing::FindExistingIdForObject(RawObject* raw_obj) {
+  ASSERT(ids_ != NULL);
+  for (int32_t i = 0; i < capacity_; i++) {
+    if (table_[i] == raw_obj) {
+      return IdOfIndex(i);
+    }
+  }
+  return kInvalidId;
+}
+
+
 RawObject* ObjectIdRing::GetObjectForId(int32_t id, LookupResult* kind) {
   int32_t index = IndexOfId(id);
   if (index == kInvalidId) {
@@ -46,6 +67,7 @@ RawObject* ObjectIdRing::GetObjectForId(int32_t id, LookupResult* kind) {
     return Object::null();
   }
   *kind = kValid;
+  ASSERT(ids_[index] == id);
   return table_[index];
 }
 
@@ -56,12 +78,41 @@ void ObjectIdRing::VisitPointers(ObjectPointerVisitor* visitor) {
 }
 
 
+void ObjectIdRing::PrintGetObjectRequestsToJSON(JSONStream* js) {

[turnidge, 2015/05/11 19:48:45]

Maybe PrintContentsToJSON or DumpToJSON?

[Cutch, 2015/05/12 14:59:57]

::PrintJSON

+  Thread* thread = Thread::Current();
+  Zone* zone = thread->zone();
+  ASSERT(zone != NULL);
+  ServiceIdZone* service_id_zone = js->service_id_zone();
+  JSONObject jsobj(js);
+  jsobj.AddProperty("type", "_ObjectIdRingRequests");

[turnidge, 2015/05/11 19:48:45]

I don't like exposing the name "Ring" in the response.  Too implementation
specific.  What is this?  The contents of a zone?  So how about "_ZoneContents"
or "_IdZoneContents" or "_ServiceIdZoneContents"?

[Cutch, 2015/05/12 14:59:57]

Acknowledged.

+  const char* request_format = "getObject?objectId=%s";

[turnidge, 2015/05/11 19:48:45]

Currently this returns a map from (request) => (response).  I would like to see
this instead be an rpc that returns the contents of a zone, so it feels like
returning a map from (id) => (response) would be better.

[Cutch, 2015/05/12 14:59:57]

Acknowledged.

+  {
+    JSONObject ring_requests(&jsobj, "ringRequests");

[turnidge, 2015/05/11 19:48:45]

In almost everyplace else in our protocol, the property names are fixed strings.
 In this case, we are using the request string as the property name.  I would
like this data to be structured differently so that it is easier to describe the
response in the IDL.

You currently return:

  ringRequests: { "getObject?objectId=objects/0" : "BLAH",
"getObject?objectId=objects/1" : "BLAH2" }

Instead I'm suggesting...

Either two parallel arrays, one of ids and one of responses

   {ids: ["objects/0", "objects/1"] responses: {"BLAH", "BLAH2"]}

Or one array of json objects,

   contents: [ { id: "objects/0", response: "BLAH" }, { id: "objects/1",
response: "BLAH2" } ]

[Cutch, 2015/05/12 14:59:57]

Acknowledged.

+    Object& obj = Object::Handle();
+    for (int32_t i = 0; i < capacity_; i++) {
+      obj = table_[i];
+      if (obj.IsNull()) {
+        // Collected object.
+        continue;
+      }
+      // Get the service id for obj.
+      const char* service_id = service_id_zone->GetServiceId(obj);
+      // Construct a fake request.
+      const char* request = zone->PrintToString(request_format, service_id);
+      // Add to ringRequests.
+      ring_requests.AddProperty(request, obj, false);
+    }
+  }
+}
+
+
 ObjectIdRing::ObjectIdRing(Isolate* isolate, int32_t capacity) {
   ASSERT(capacity > 0);
   isolate_ = isolate;
   serial_num_ = 0;
   wrapped_ = false;
   table_ = NULL;
+  ids_ = NULL;
   SetCapacityAndMaxSerial(capacity, kMaxId);
 }
 
@@ -74,9 +125,14 @@ void ObjectIdRing::SetCapacityAndMaxSerial(int32_t capacity,
     free(table_);
   }
   table_ = reinterpret_cast<RawObject**>(calloc(capacity_, kWordSize));
-  for (int i = 0; i < capacity_; i++) {
+  for (int32_t i = 0; i < capacity_; i++) {
     table_[i] = Object::null();
   }
+  // Allocate buffer to remember ids.
+  ids_ = reinterpret_cast<int32_t*>(calloc(capacity_, sizeof(int32_t)));
+  for (int32_t i = 0; i < capacity_; i++) {
+    ids_[i] = kInvalidId;
+  }
   // The maximum serial number is a multiple of the capacity, so that when
   // the serial number wraps, the index into table_ wraps with it.
   max_serial_ = max_serial - (max_serial % capacity_);
@@ -98,14 +154,13 @@ int32_t ObjectIdRing::AllocateNewId(RawObject* raw_obj) {
   ASSERT(raw_obj->IsHeapObject());
   int32_t id = NextSerial();
   ASSERT(id != kInvalidId);
-  int32_t cursor = IndexOfId(id);
-  ASSERT(cursor != kInvalidId);
-  if (table_[cursor] != Object::null()) {
-    // Free old handle.
-    table_[cursor] = Object::null();
-  }
-  ASSERT(table_[cursor] == Object::null());
-  table_[cursor] = raw_obj;
+  int32_t index = IndexOfId(id);
+  ASSERT(index != kInvalidId);
+  table_[index] = raw_obj;
+  if (ids_ != NULL) {
+    // Remember id.
+    ids_[index] = id;
+  }
   return id;
 }
 
@@ -119,6 +174,14 @@ int32_t ObjectIdRing::IndexOfId(int32_t id) {
 }
 
 
+int32_t ObjectIdRing::IdOfIndex(int32_t index) {
+  ASSERT(ids_ != NULL);
+  int32_t id = ids_[index];
+  ASSERT(IsValidId(id));
+  return id;
+}
+
+
 bool ObjectIdRing::IsValidContiguous(int32_t id) {
   ASSERT(id != kInvalidId);
   ASSERT((id >= 0) && (id < max_serial_));