Refactor VM service IDs

runtime/vm/object.cc

Index: runtime/vm/object.cc
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
index b70fffb0b85ad23b702371b46461e04c03a342a3..6a1c1be85407a95311f4b4ada1f0d36b2fcb09c7 100644
--- a/runtime/vm/object.cc
+++ b/runtime/vm/object.cc
@@ -1808,6 +1808,32 @@ void Class::AddFunction(const Function& function) const {
 }
 
 
+intptr_t Class::FindFunctionIndex(const Function& needle) const {
+  Isolate* isolate = Isolate::Current();
+  if (EnsureIsFinalized(isolate) != Error::null()) {
+    return -1;
+  }
+  ReusableHandleScope reused_handles(isolate);
+  Array& funcs = reused_handles.ArrayHandle();
+  funcs ^= functions();
+  ASSERT(!funcs.IsNull());
+  Function& function = reused_handles.FunctionHandle();
+  String& function_name = reused_handles.StringHandle();
+  String& needle_name = String::Handle();

[turnidge, 2013/12/19 20:39:29]

String::Handle(isolate) maybe?  Avoids a call to Isolate::Current().

[Cutch, 2013/12/19 21:53:58]

Done.

+  needle_name ^= needle.name();
+  const intptr_t len = funcs.Length();
+  for (intptr_t i = 0; i < len; i++) {
+    function ^= funcs.At(i);
+    function_name ^= function.name();
+    if (function_name.Equals(needle_name)) {
+      return i;
+    }
+  }
+  // No function found.
+  return -1;
+}
+
+
 void Class::AddClosureFunction(const Function& function) const {
   GrowableObjectArray& closures =
       GrowableObjectArray::Handle(raw_ptr()->closure_functions_);
@@ -1848,6 +1874,29 @@ RawFunction* Class::LookupClosureFunction(intptr_t token_pos) const {
   return closure.raw();
 }
 
+intptr_t Class::FindClosureIndex(intptr_t token_pos) const {
+  if (raw_ptr()->closure_functions_ == GrowableObjectArray::null()) {
+    return -1;
+  }

[turnidge, 2013/12/19 20:39:29]

Do you want to use the ReusableHandleScope here for consistency?

[Cutch, 2013/12/19 21:53:58]

Done.

+  const GrowableObjectArray& closures =
+      GrowableObjectArray::Handle(raw_ptr()->closure_functions_);
+  Function& closure = Function::Handle();
+  intptr_t num_closures = closures.Length();
+  intptr_t best_fit_token_pos = -1;
+  intptr_t best_fit_index = -1;
+  for (intptr_t i = 0; i < num_closures; i++) {
+    closure ^= closures.At(i);
+    ASSERT(!closure.IsNull());
+    if ((closure.token_pos() <= token_pos) &&
+        (token_pos <= closure.end_token_pos()) &&
+        (best_fit_token_pos < closure.token_pos())) {
+      best_fit_index = i;
+      best_fit_token_pos = closure.token_pos();
+    }
+  }
+  return best_fit_index;
+}
+
 
 void Class::set_signature_function(const Function& value) const {
   ASSERT(value.IsClosureFunction() || value.IsSignatureFunction());
@@ -2399,6 +2448,32 @@ void Class::SetFields(const Array& value) const {
 }
 
 
+intptr_t Class::FindFieldIndex(const Field& needle) const {
+  Isolate* isolate = Isolate::Current();
+  if (EnsureIsFinalized(isolate) != Error::null()) {
+    return -1;
+  }
+  ReusableHandleScope reused_handles(isolate);
+  Array& fields_array = reused_handles.ArrayHandle();
+  fields_array ^= fields();
+  ASSERT(!fields_array.IsNull());
+  Field& field = reused_handles.FieldHandle();
+  String& field_name = reused_handles.StringHandle();
+  String& needle_name = String::Handle();

[turnidge, 2013/12/19 20:39:29]

String::Handle(isolate)

Do we run into this a lot?  It would be possible to add a StringHandle2 accessor
(hand-written) to ReusableHandleScope.

[Cutch, 2013/12/19 21:53:58]

Depends on user activity. We hit this whenever we need to generate an ID for a
field. 

[turnidge, 2013/12/20 17:44:45]

I meant do we run into needing two String handles (as opposed to just the one)
in a leaf function often while coding.  Doesn't matter.  We can optimize that
case if it shows on profiles.

+  needle_name ^= needle.name();
+  const intptr_t len = fields_array.Length();
+  for (intptr_t i = 0; i < len; i++) {
+    field ^= fields_array.At(i);
+    field_name ^= field.name();
+    if (field_name.Equals(needle_name)) {
+      return i;
+    }
+  }
+  // No field found found.
+  return -1;
+}
+
+
 template <class FakeInstance>
 RawClass* Class::New(intptr_t index) {
   ASSERT(Object::class_class() != Class::null());
@@ -3248,7 +3323,7 @@ void Class::PrintToJSONStream(JSONStream* stream, bool ref) const {
   const char* user_visible_class_name =
       String::Handle(UserVisibleName()).ToCString();
   jsobj.AddProperty("type", JSONType(ref));
-  jsobj.AddProperty("id", id());
+  jsobj.AddPropertyF("id", "classes/%" Pd "", id());
   jsobj.AddProperty("name", internal_class_name);
   jsobj.AddProperty("user_name", user_visible_class_name);
   if (!ref) {
@@ -5619,11 +5694,26 @@ void Function::PrintToJSONStream(JSONStream* stream, bool ref) const {
   const char* internal_function_name = String::Handle(name()).ToCString();
   const char* function_name =
       String::Handle(QualifiedUserVisibleName()).ToCString();
-  ObjectIdRing* ring = Isolate::Current()->object_id_ring();
-  intptr_t id = ring->GetIdForObject(raw());
+  Class& cls = Class::Handle(Owner());
+  Error& err = Error::Handle();
+  err ^= cls.EnsureIsFinalized(Isolate::Current());
+  ASSERT(err.IsNull());
+  Function& func = Function::Handle(raw());

[turnidge, 2013/12/19 20:39:29]

Instead of:

Function::Handle(raw())

Can we get away with:

*this

?

+  intptr_t id = cls.FindFunctionIndex(func);
+  bool closure = false;
+  if (id == -1) {
+    closure = true;

[turnidge, 2013/12/19 20:39:29]

There is an IsClosureFunction predicate in Function.  Would that work to
distinguish these cases without hunting through the functions array?

[Cutch, 2013/12/19 21:53:58]

Possibly, let me test.

+    id = cls.FindClosureIndex(token_pos());
+  }
+  ASSERT(id >= 0);
+  intptr_t cid = cls.id();
   JSONObject jsobj(stream);
   jsobj.AddProperty("type", JSONType(ref));
-  jsobj.AddProperty("id", id);
+  if (closure) {
+    jsobj.AddPropertyF("id", "classes/%" Pd "/closures/%" Pd "", cid, id);

[turnidge, 2013/12/19 20:39:29]

For thought: I'm wondering if these format strings (or paths or whatever) should
be gathered in a central place somehow.  My idea is that the same string
constant could be used in service dispatching and in link construction so they
don't get out of sync.

For now, this works.

[Cutch, 2013/12/19 21:53:58]

Agreed.

+  } else {
+    jsobj.AddPropertyF("id", "classes/%" Pd "/functions/%" Pd "", cid, id);
+  }
   jsobj.AddProperty("name", internal_function_name);
   jsobj.AddProperty("user_name", function_name);
   if (ref) return;
@@ -5952,10 +6042,12 @@ void Field::PrintToJSONStreamWithInstance(JSONStream* stream,
   JSONObject jsobj(stream);
   const char* internal_field_name = String::Handle(name()).ToCString();
   const char* field_name = String::Handle(UserVisibleName()).ToCString();
-  ObjectIdRing* ring = Isolate::Current()->object_id_ring();
-  intptr_t id = ring->GetIdForObject(raw());
+  Class& cls = Class::Handle(owner());
+  intptr_t id = cls.FindFieldIndex(Field::Handle(raw()));

[turnidge, 2013/12/19 20:39:29]

*this

[Cutch, 2013/12/19 21:53:58]

Done.

+  ASSERT(id >= 0);
+  intptr_t cid = cls.id();
   jsobj.AddProperty("type", JSONType(ref));
-  jsobj.AddProperty("id", id);
+  jsobj.AddPropertyF("id", "classes/%" Pd "/fields/%" Pd "", cid, id);
   jsobj.AddProperty("name", internal_field_name);
   jsobj.AddProperty("user_name", field_name);
   if (is_static()) {
@@ -5965,7 +6057,7 @@ void Field::PrintToJSONStreamWithInstance(JSONStream* stream,
     const Object& valueObj = Object::Handle(instance.GetField(*this));
     jsobj.AddProperty("value", valueObj);
   }
-  Class& cls = Class::Handle(owner());
+
   jsobj.AddProperty("owner", cls);
   AbstractType& declared_type = AbstractType::Handle(type());
   cls = declared_type.type_class();
@@ -7119,11 +7211,12 @@ const char* Script::ToCString() const {
 
 void Script::PrintToJSONStream(JSONStream* stream, bool ref) const {
   JSONObject jsobj(stream);
-  ObjectIdRing* ring = Isolate::Current()->object_id_ring();
-  intptr_t id = ring->GetIdForObject(raw());
   jsobj.AddProperty("type", JSONType(ref));
-  jsobj.AddProperty("id", id);
   const String& name = String::Handle(url());
+  ASSERT(!name.IsNull());
+  const String& encoded_url = String::Handle(String::EncodeURI(name));
+  ASSERT(!encoded_url.IsNull());
+  jsobj.AddPropertyF("id", "scripts/%s", encoded_url.ToCString());
   jsobj.AddProperty("name", name.ToCString());
   jsobj.AddProperty("kind", GetKindAsCString());
   if (ref) {
@@ -8193,13 +8286,13 @@ const char* Library::ToCString() const {
 void Library::PrintToJSONStream(JSONStream* stream, bool ref) const {
   const char* library_name = String::Handle(name()).ToCString();
   const char* library_url = String::Handle(url()).ToCString();
-  ObjectIdRing* ring = Isolate::Current()->object_id_ring();
-  intptr_t id = ring->GetIdForObject(raw());
+  intptr_t id = index();
+  ASSERT(id >= 0);
   JSONObject jsobj(stream);
   jsobj.AddProperty("type", JSONType(ref));
-  jsobj.AddProperty("id", id);
+  jsobj.AddPropertyF("id", "libraries/%" Pd "", id);
   jsobj.AddProperty("name", library_name);
-  jsobj.AddProperty("url", library_url);
+  jsobj.AddProperty("user_name", library_url);
   if (ref) return;
   {
     JSONArray jsarr(&jsobj, "classes");
@@ -9769,13 +9862,18 @@ void Code::PrintToJSONStream(JSONStream* stream, bool ref) const {
   ObjectIdRing* ring = Isolate::Current()->object_id_ring();
   intptr_t id = ring->GetIdForObject(raw());
   JSONObject jsobj(stream);
+  jsobj.AddProperty("type", JSONType(ref));
+  jsobj.AddPropertyF("id", "objects/%" Pd "", id);
+  if (is_optimized()) {
+    jsobj.AddProperty("name", "optimized code");
+    jsobj.AddProperty("user_name", "optimized code");
+  } else {
+    jsobj.AddProperty("name", "regular code");
+    jsobj.AddProperty("user_name", "regular code");

[turnidge, 2013/12/19 20:39:29]

Too bad we can't get a nice name here.  I wonder...

[Cutch, 2013/12/19 21:53:58]

Done.

+  }
   if (ref) {
-    jsobj.AddProperty("type", "@Code");
-    jsobj.AddProperty("id", id);
     return;
   }
-  jsobj.AddProperty("type", "Code");
-  jsobj.AddProperty("id", id);
   jsobj.AddProperty("is_optimized", is_optimized());
   jsobj.AddProperty("is_alive", is_alive());
   jsobj.AddProperty("function", Object::Handle(function()));
@@ -11477,7 +11575,7 @@ void Instance::PrintToJSONStream(JSONStream* stream, bool ref) const {
 
   JSONObject jsobj(stream);
   jsobj.AddProperty("type", JSONType(ref));
-  jsobj.AddProperty("id", id);
+  jsobj.AddPropertyF("id", "objects/%" Pd "", id);
 
   Class& cls = Class::Handle(this->clazz());
   jsobj.AddProperty("class", cls);
@@ -14335,6 +14433,28 @@ RawString* String::EscapeSpecialCharacters(const String& str) {
 }
 
 
+RawString* String::EncodeURI(const String& str) {
+  if (str.IsOneByteString()) {
+    return OneByteString::EncodeURI(str);
+  }
+  if (str.IsExternalOneByteString()) {
+    return ExternalOneByteString::EncodeURI(str);
+  }
+  return String::null();

[turnidge, 2013/12/19 20:39:29]

Is this case UNREACHABLE or UNIMPLEMENTED?

[Cutch, 2013/12/19 21:53:58]

Added UNREACHABLE();

+}
+
+
+RawString* String::DecodeURI(const String& str) {
+  if (str.IsOneByteString()) {
+    return OneByteString::DecodeURI(str);
+  }
+  if (str.IsExternalOneByteString()) {
+    return ExternalOneByteString::DecodeURI(str);
+  }
+  return String::null();
+}
+
+
 RawString* String::NewFormatted(const char* format, ...) {
   va_list args;
   va_start(args, format);
@@ -14892,6 +15012,175 @@ RawOneByteString* ExternalOneByteString::EscapeSpecialCharacters(
 }
 
 
+static bool IsPercent(int32_t c) {
+  return c == '%';
+}
+
+static bool IsURISafeCharacter(int32_t c) {
+  if ((c >= '0') && (c <= '9')) {
+    return true;
+  }
+  if ((c >= 'a') && (c <= 'z')) {
+    return true;
+  }
+  if ((c >= 'A') && (c <= 'Z')) {
+    return true;
+  }
+  return (c == '-') || (c == '_') || (c == '.') || (c == '~');
+}
+
+
+static int32_t GetHexCharacter(int32_t c) {
+  ASSERT(c >= 0);
+  ASSERT(c < 16);
+  const char* hex = "0123456789ABCDEF";
+  return hex[c];
+}
+
+
+static int32_t GetHexValue(int32_t c) {
+  if (c >= '0' && c <= '9') {
+    return c - '0';
+  }
+  if (c >= 'A' && c <= 'F') {
+    return c - 'A' + 10;
+  }
+  UNREACHABLE();
+  return 0;
+}
+
+
+static int32_t MergeHexCharacters(int32_t c1, int32_t c2) {
+  return GetHexValue(c1) << 4 | GetHexValue(c2);
+}
+
+
+RawOneByteString* OneByteString::EncodeURI(const String& str) {
+  intptr_t len = str.Length();
+  if (len > 0) {
+    intptr_t num_escapes = 0;
+    for (intptr_t i = 0; i < len; i++) {
+      if (!IsURISafeCharacter(*CharAddr(str, i))) {
+        num_escapes += 2;
+      }
+    }
+    const String& dststr = String::Handle(
+        OneByteString::New(len + num_escapes, Heap::kNew));
+    intptr_t index = 0;
+    for (intptr_t i = 0; i < len; i++) {
+      if (!IsURISafeCharacter(*CharAddr(str, i))) {
+        *(CharAddr(dststr, index)) = '%';
+        int32_t ch = *CharAddr(str, i);
+        *(CharAddr(dststr, index + 1)) = GetHexCharacter(ch >> 4);
+        *(CharAddr(dststr, index + 2)) = GetHexCharacter(ch & 0xF);
+        index += 3;
+      } else {
+        *(CharAddr(dststr, index)) = *CharAddr(str, i);
+        index += 1;
+      }
+    }
+    return OneByteString::raw(dststr);
+  }
+  return OneByteString::raw(Symbols::Empty());
+}
+
+
+RawOneByteString* ExternalOneByteString::EncodeURI(const String& str) {
+  intptr_t len = str.Length();
+  if (len > 0) {
+    intptr_t num_escapes = 0;
+    for (intptr_t i = 0; i < len; i++) {
+      if (!IsURISafeCharacter(*CharAddr(str, i))) {
+        num_escapes += 2;
+      }
+    }
+    const String& dststr = String::Handle(
+        OneByteString::New(len + num_escapes, Heap::kNew));
+    intptr_t index = 0;
+    for (intptr_t i = 0; i < len; i++) {
+      if (!IsURISafeCharacter(*CharAddr(str, i))) {
+        *(OneByteString::CharAddr(dststr, index)) = '%';
+        int32_t ch = *CharAddr(str, i);
+        *(OneByteString::CharAddr(dststr, index + 1)) =
+            GetHexCharacter(ch >> 4);
+        *(OneByteString::CharAddr(dststr, index + 2)) =
+            GetHexCharacter(ch & 0xF);
+        index += 3;
+      } else {
+        *(OneByteString::CharAddr(dststr, index)) = *CharAddr(str, i);
+        index += 1;
+      }
+    }
+    return OneByteString::raw(dststr);
+  }
+  return OneByteString::raw(Symbols::Empty());
+}

[turnidge, 2013/12/19 20:39:29]

Is there a way to avoid the code duplication by having a helper function that
operates on a String?  Would it help if we weren't putting the characters into
the destination string directly but instead used a temp buffer?

For example, String::EscapedString (in this file) is able to use a
CodePointIterator and works for all String types.

I would rather not have two versions of the Encode/Decode here.

[Cutch, 2013/12/19 21:53:58]

Percent encoding is only defined on OneByteStrings, let's discuss.

+
+
+RawOneByteString* OneByteString::DecodeURI(const String& str) {
+  intptr_t len = str.Length();
+  if (len > 0) {
+    intptr_t num_escapes = 0;
+    for (intptr_t i = 0; i < len; i++) {
+      if (IsPercent(*CharAddr(str, i))) {
+        num_escapes += 2;
+      }
+    }
+    ASSERT(len - num_escapes > 0);
+    const String& dststr = String::Handle(
+        OneByteString::New(len - num_escapes, Heap::kNew));
+    intptr_t index = 0;
+    for (intptr_t i = 0; i < len;) {
+      if (IsPercent(*CharAddr(str, i))) {
+        int32_t ch1 = *CharAddr(str, i + 1);
+        int32_t ch2 = *CharAddr(str, i + 2);
+        int32_t merged = MergeHexCharacters(ch1, ch2);
+        *(CharAddr(dststr, index)) = merged;
+        i += 3;
+      } else {
+        *(CharAddr(dststr, index)) = *CharAddr(str, i);
+        i += 1;
+      }
+      index++;
+    }
+    return OneByteString::raw(dststr);
+  }
+  return OneByteString::raw(Symbols::Empty());
+}
+
+
+RawOneByteString* ExternalOneByteString::DecodeURI(const String& str) {
+  intptr_t len = str.Length();
+  if (len > 0) {
+    intptr_t num_escapes = 0;
+    for (intptr_t i = 0; i < len; i++) {
+      if (IsPercent(*CharAddr(str, i))) {
+        num_escapes += 2;
+      }
+    }
+    ASSERT(len - num_escapes > 0);
+    const String& dststr = String::Handle(
+        OneByteString::New(len - num_escapes, Heap::kNew));
+    intptr_t index = 0;
+    for (intptr_t i = 0; i < len;) {
+      if (IsPercent(*CharAddr(str, i))) {
+        int32_t ch1 = *CharAddr(str, i + 1);
+        int32_t ch2 = *CharAddr(str, i + 2);
+        int32_t merged = MergeHexCharacters(ch1, ch2);
+        *(OneByteString::CharAddr(dststr, index)) = merged;
+        i += 3;
+      } else {
+        *(OneByteString::CharAddr(dststr, index)) = *CharAddr(str, i);
+        i += 1;
+      }
+      index++;
+    }
+    return OneByteString::raw(dststr);
+  }
+  return OneByteString::raw(Symbols::Empty());
+}
+
+
 RawOneByteString* OneByteString::New(intptr_t len,
                                      Heap::Space space) {
   ASSERT(Isolate::Current() == Dart::vm_isolate() ||