optimize and implement enum/message/service descriptor

lookups by name and value.


git-svn-id: https://protobuf-c.googlecode.com/svn/trunk@51 00440858-1255-0410-a3e6-75ea37f81c3a

TODO

@@ -1,5 +1,9 @@
 - provide example rpc
 - at least include extensions in Descriptor
+- test code:
+  - service method lookups
+  - out-of-order fields in messages (ie if the number isn't ascending)
+  - gaps in numbers: check that the number of ranges is correct
 
 - Documentations:
   - __INIT macro

src/google/protobuf-c/protobuf-c-private.h

@@ -7,6 +7,12 @@ void protobuf_c_buffer_simple_append (ProtobufCBuffer *buffer,
 
 /* === stuff which needs to be declared for use in the generated code === */
 
+struct _ProtobufCEnumValueIndex
+{
+  const char *name;
+  unsigned index;               /* into values[] array */
+};
+
 /* IntRange: helper structure for optimizing
      int => index lookups
    in the case where the keys are mostly consecutive values,

src/google/protobuf-c/protobuf-c.c

@@ -1568,3 +1568,114 @@ void protobuf_c_service_destroy (ProtobufCService *service)
 {
   service->destroy (service);
 }
+
+/* --- querying the descriptors --- */
+const ProtobufCEnumValue *
+protobuf_c_enum_descriptor_get_value_by_name 
+                         (const ProtobufCEnumDescriptor    *desc,
+                          const char                       *name)
+{
+  unsigned start = 0, count = desc->n_value_names;
+  while (count > 1)
+    {
+      unsigned mid = start + count / 2;
+      int rv = strcmp (desc->values_by_name[mid].name, name);
+      if (rv == 0)
+        return desc->values + desc->values_by_name[mid].index;
+      else if (rv < 0)
+        {
+          count = start + count - (mid - 1);
+          start = mid + 1;
+        }
+      else
+        count = mid - start;
+    }
+  if (count == 0)
+    return NULL;
+  if (strcmp (desc->values_by_name[start].name, name) == 0)
+    return desc->values + desc->values_by_name[start].index;
+  return NULL;
+}
+const ProtobufCEnumValue *
+protobuf_c_enum_descriptor_get_value        
+                         (const ProtobufCEnumDescriptor    *desc,
+                          int                               value)
+{
+  int rv = int_range_lookup (desc->n_value_ranges, desc->value_ranges, value);
+  if (rv < 0)
+    return NULL;
+  return desc->values + rv;
+}
+
+const ProtobufCFieldDescriptor *
+protobuf_c_message_descriptor_get_field_by_name
+                         (const ProtobufCMessageDescriptor *desc,
+                          const char                       *name)
+{
+  unsigned start = 0, count = desc->n_fields;
+  const ProtobufCFieldDescriptor *field;
+  while (count > 1)
+    {
+      unsigned mid = start + count / 2;
+      int rv;
+      field = desc->fields + desc->fields_sorted_by_name[mid];
+      rv = strcmp (field->name, name);
+      if (rv == 0)
+        return field;
+      else if (rv < 0)
+        {
+          count = start + count - (mid + 1);
+          start = mid + 1;
+        }
+      else
+        count = mid - start;
+    }
+  if (count == 0)
+    return NULL;
+  field = desc->fields + desc->fields_sorted_by_name[start];
+  if (strcmp (field->name, name) == 0)
+    return field;
+  return NULL;
+}
+
+const ProtobufCFieldDescriptor *
+protobuf_c_message_descriptor_get_field        
+                         (const ProtobufCMessageDescriptor *desc,
+                          unsigned                          value)
+{
+  int rv = int_range_lookup (desc->n_field_ranges,
+                             desc->field_ranges,
+                             value);
+  if (rv < 0)
+    return NULL;
+  return desc->fields + rv;
+}
+
+const ProtobufCMethodDescriptor *
+protobuf_c_service_descriptor_get_method_by_name
+                         (const ProtobufCServiceDescriptor *desc,
+                          const char                       *name)
+{
+  unsigned start = 0, count = desc->n_methods;
+  while (count > 1)
+    {
+      unsigned mid = start + count / 2;
+      int rv = strcmp (desc->methods[mid].name, name);
+      if (rv == 0)
+        return desc->methods + mid;
+      if (rv < 0)
+        {
+          count = start + count - (mid - 1);
+          start = mid + 1;
+        }
+      else
+        {
+          count = mid - start;
+        }
+    }
+  if (count == 0)
+    return NULL;
+  if (strcmp (desc->methods[start].name, name) == 0)
+    return desc->methods + start;
+  return NULL;
+}

src/google/protobuf-c/protobuf-c.h

@@ -84,6 +84,7 @@ struct _ProtobufCBuffer
 };
 /* --- enums --- */
 typedef struct _ProtobufCEnumValue ProtobufCEnumValue;
+typedef struct _ProtobufCEnumValueIndex ProtobufCEnumValueIndex;
 typedef struct _ProtobufCEnumDescriptor ProtobufCEnumDescriptor;
 
 struct _ProtobufCEnumValue
@@ -108,7 +109,11 @@ struct _ProtobufCEnumDescriptor
 
   /* sorted by name */
   unsigned n_value_names;
-  const ProtobufCEnumValue *values_by_name;
+  const ProtobufCEnumValueIndex *values_by_name;
+
+  /* value-ranges, for faster lookups by number */
+  unsigned n_value_ranges;
+  const ProtobufCIntRange *value_ranges;
 };
 
 /* --- messages --- */
@@ -138,12 +143,14 @@ struct _ProtobufCMessageDescriptor
   /* sorted by field-id */
   unsigned n_fields;
   const ProtobufCFieldDescriptor *fields;
+  const unsigned *fields_sorted_by_name;
 
   /* ranges, optimization for looking up fields */
   unsigned n_field_ranges;
   const ProtobufCIntRange *field_ranges;
 };
 
+
 typedef struct _ProtobufCMessage ProtobufCMessage;
 typedef struct _ProtobufCMessageUnknownField ProtobufCMessageUnknownField;
 struct _ProtobufCMessage
@@ -209,6 +216,27 @@ struct _ProtobufCService
 void protobuf_c_service_destroy (ProtobufCService *);
 
 
+/* --- querying the descriptors --- */
+const ProtobufCEnumValue *
+protobuf_c_enum_descriptor_get_value_by_name 
+                         (const ProtobufCEnumDescriptor    *desc,
+                          const char                       *name);
+const ProtobufCEnumValue *
+protobuf_c_enum_descriptor_get_value        
+                         (const ProtobufCEnumDescriptor    *desc,
+                          int                               value);
+const ProtobufCFieldDescriptor *
+protobuf_c_message_descriptor_get_field_by_name
+                         (const ProtobufCMessageDescriptor *desc,
+                          const char                       *name);
+const ProtobufCFieldDescriptor *
+protobuf_c_message_descriptor_get_field        
+                         (const ProtobufCMessageDescriptor *desc,
+                          unsigned                          value);
+const ProtobufCMethodDescriptor *
+protobuf_c_service_descriptor_get_method_by_name
+                         (const ProtobufCServiceDescriptor *desc,
+                          const char                       *name);
 
 /* --- wire format enums --- */
 typedef enum

src/google/protobuf/compiler/c/c_enum.cc

@@ -85,15 +85,12 @@ void EnumGenerator::GenerateDescriptorDeclarations(io::Printer* printer) {
     "extern $dllexport$const ProtobufCEnumDescriptor    $lcclassname$__descriptor;\n");
 }
 
-struct NameIndex
-{
-  const char *name;
-  unsigned index;
-};
 struct ValueIndex
 {
   int value;
   unsigned index;
+  unsigned final_index;		/* index in uniqified array of values */
+  const char *name;
 };
 void EnumGenerator::GenerateValueInitializer(io::Printer *printer, int index)
 {
@@ -106,12 +103,6 @@ void EnumGenerator::GenerateValueInitializer(io::Printer *printer, int index)
    "  { \"$enum_value_name$\", \"$c_enum_value_name$\", $value$ },\n");
 }
 
-static int compare_name_indices_by_name(const void *a, const void *b)
-{
-  const NameIndex *ni_a = (const NameIndex *) a;
-  const NameIndex *ni_b = (const NameIndex *) b;
-  return strcmp (ni_a->name, ni_b->name);
-}
 static int compare_value_indices_by_value_then_index(const void *a, const void *b)
 {
   const ValueIndex *vi_a = (const ValueIndex *) a;
@@ -123,6 +114,13 @@ static int compare_value_indices_by_value_then_index(const void *a, const void *
   return 0;
 }
 
+static int compare_value_indices_by_name(const void *a, const void *b)
+{
+  const ValueIndex *vi_a = (const ValueIndex *) a;
+  const ValueIndex *vi_b = (const ValueIndex *) b;
+  return strcmp (vi_a->name, vi_b->name);
+}
+
 void EnumGenerator::GenerateEnumDescriptor(io::Printer* printer) {
   map<string, string> vars;
   vars["fullname"] = descriptor_->full_name();
@@ -142,9 +140,8 @@ void EnumGenerator::GenerateEnumDescriptor(io::Printer* printer) {
     name_index[j].name = vd->name().c_str();
     value_index[j].index = j;
     value_index[j].value = vd->number();
+    value_index[j].name = vd->name().c_str();
   }
-  qsort(name_index, descriptor_->value_count(),
-	sizeof(NameIndex), compare_name_indices_by_name);
   qsort(value_index, descriptor_->value_count(),
 	sizeof(ValueIndex), compare_value_indices_by_value_then_index);
 
@@ -154,15 +151,18 @@ void EnumGenerator::GenerateEnumDescriptor(io::Printer* printer) {
     n_unique_values = 0; // should never happen
   } else {
     n_unique_values = 1;
+    value_index[0].final_index = 0;
     for (int j = 1; j < descriptor_->value_count(); j++) {
       if (value_index[j-1].value != value_index[j].value)
-	value_index[n_unique_values++] = value_index[j];
+	value_index[j].final_index = n_unique_values++;
+      else
+	value_index[j].final_index = n_unique_values - 1;
     }
   }
 
   vars["unique_value_count"] = SimpleItoa(n_unique_values);
   printer->Print(vars,
-    "const ProtobufCEnumValue $lcclassname$_enum_values_by_number[$unique_value_count$] =\n"
+    "const ProtobufCEnumValue $lcclassname$__enum_values_by_number[$unique_value_count$] =\n"
     "{\n");
   if (descriptor_->value_count() > 0) {
     GenerateValueInitializer(printer, value_index[0].index);
@@ -173,12 +173,54 @@ void EnumGenerator::GenerateEnumDescriptor(io::Printer* printer) {
     }
   }
   printer->Print(vars, "};\n");
+  printer->Print(vars, "static const ProtobufCIntRange $lcclassname$__value_ranges[] = {\n");
+  unsigned n_ranges = 0;
+  if (descriptor_->value_count() > 0) {
+    unsigned range_start = 0;
+    unsigned range_len = 1;
+    unsigned range_start_value = value_index[0].value;
+    unsigned last_value = range_start_value;
+    for (int j = 1; j < descriptor_->value_count(); j++) {
+      if (value_index[j-1].value != value_index[j].value) {
+	if (last_value + 1 == value_index[j].value) {
+	  range_len++;
+	} else {
+	  // output range
+	  vars["range_start_value"] = SimpleItoa(range_start_value);
+	  vars["orig_index"] = SimpleItoa(range_start);
+	  printer->Print (vars, "{$range_start_value$, $orig_index$},");
+	  range_start_value = value_index[j].value;
+	  range_start += range_len;
+	  range_len = 1;
+	  n_ranges++;
+	}
+	last_value = value_index[j].value;
+      }
+    }
+    {
+    vars["range_start_value"] = SimpleItoa(range_start_value);
+    vars["orig_index"] = SimpleItoa(range_start);
+    printer->Print (vars, "{$range_start_value$, $orig_index$},");
+    range_start += range_len;
+    n_ranges++;
+    }
+    {
+    vars["range_start_value"] = SimpleItoa(0);
+    vars["orig_index"] = SimpleItoa(range_start);
+    printer->Print (vars, "{$range_start_value$, $orig_index$}\n};\n");
+    }
+  }
+  vars["n_ranges"] = SimpleItoa(n_ranges);
 
+  qsort(value_index, descriptor_->value_count(),
+        sizeof(ValueIndex), compare_value_indices_by_name);
   printer->Print(vars,
-    "const ProtobufCEnumValue $lcclassname$_enum_values_by_name[$value_count$] =\n"
+    "const ProtobufCEnumValueIndex $lcclassname$__enum_values_by_name[$value_count$] =\n"
     "{\n");
   for (int j = 0; j < descriptor_->value_count(); j++) {
-    GenerateValueInitializer(printer, name_index[j].index);
+    vars["index"] = SimpleItoa(value_index[j].final_index);
+    vars["name"] = value_index[j].name;
+    printer->Print (vars, "  { \"$name$\", $index$ },\n");
   }
   printer->Print(vars, "};\n");
 
@@ -191,9 +233,11 @@ void EnumGenerator::GenerateEnumDescriptor(io::Printer* printer) {
     "  \"$cname$\",\n"
     "  \"$packagename$\",\n"
     "  $unique_value_count$,\n"
-    "  $lcclassname$_enum_values_by_number,\n"
+    "  $lcclassname$__enum_values_by_number,\n"
     "  $value_count$,\n"
-    "  $lcclassname$_enum_values_by_name\n"
+    "  $lcclassname$__enum_values_by_name,\n"
+    "  $n_ranges$,\n"
+    "  $lcclassname$__value_ranges\n"
     "};\n");
 }
 

src/google/protobuf/compiler/c/c_helpers.cc

@@ -152,6 +152,13 @@ string ConvertToSpaces(const string &input) {
   return string(input.size(), ' ');
 }
 
+int compare_name_indices_by_name(const void *a, const void *b)
+{
+  const NameIndex *ni_a = (const NameIndex *) a;
+  const NameIndex *ni_b = (const NameIndex *) b;
+  return strcmp (ni_a->name, ni_b->name);
+}
+
 
 string CEscape(const string& src);
 
@@ -273,7 +280,7 @@ WriteIntRanges(io::Printer* printer, int n_values, const int *values, const stri
         n_ranges++;
     }
     vars["n_ranges"] = SimpleItoa(n_ranges);
-    printer->Print(vars, "static ProtobufCIntRange $name$[$n_ranges$ + 1] =\n"
+    printer->Print(vars, "static const ProtobufCIntRange $name$[$n_ranges$ + 1] =\n"
                          "{\n");
     int last_range_start = 0;
     for (int i = 1; i < n_values; i++) {

src/google/protobuf/compiler/c/c_helpers.h

@@ -122,6 +122,13 @@ string GetLabelName(FieldDescriptor::Label label);
 // returns the number of ranges there are to bsearch.
 unsigned WriteIntRanges(io::Printer* printer, int n_values, const int *values, const string &name);
 
+struct NameIndex
+{
+  unsigned index;
+  const char *name;
+};
+int compare_name_indices_by_name(const void*, const void*);
+
 }  // namespace c
 }  // namespace compiler
 }  // namespace protobuf

src/google/protobuf/compiler/c/c_message.cc

@@ -280,6 +280,21 @@ GenerateMessageDescriptor(io::Printer* printer) {
   printer->Print(vars,
     "};\n");
 
+  NameIndex *field_indices = new NameIndex [descriptor_->field_count()];
+  for (int i = 0; i < descriptor_->field_count(); i++) {
+    field_indices[i].name = sorted_fields[i]->name().c_str();
+    field_indices[i].index = i;
+  }
+  qsort (field_indices, descriptor_->field_count(), sizeof (NameIndex),
+         compare_name_indices_by_name);
+  printer->Print(vars, "static const unsigned $lcclassname$__field_indices_by_name[] = {\n");
+  for (int i = 0; i < descriptor_->field_count(); i++) {
+    vars["index"] = SimpleItoa(field_indices[i].index);
+    vars["name"] = field_indices[i].name;
+    printer->Print(vars, "  $index$,   /* field[$index$] = $name$ */\n");
+  }
+  printer->Print("};\n");
+
   // create range initializers
   int *values = new int[descriptor_->field_count()];
   for (int i = 0; i < descriptor_->field_count(); i++) {
@@ -303,6 +318,7 @@ GenerateMessageDescriptor(io::Printer* printer) {
     "  sizeof($classname$),\n"
     "  $n_fields$,\n"
     "  $lcclassname$__field_descriptors,\n"
+    "  $lcclassname$__field_indices_by_name,\n"
     "  $n_ranges$,"
     "  $lcclassname$__number_ranges\n"
     "};\n");

src/simplerpc/simplerpc.c

@@ -85,8 +85,10 @@ struct _BuiltinService
 static void
 builtin_service_list_domains (Simplerpc__Builtin_Service *service,
                               const Simplerpc__DomainListRequest *input,
-                              ProtobufCClosure *closure)
+                              Simplerpc__DomainListResponse__ClosureFunc closure,
+                              void *closure_data)
 {
+  Simplerpc__DomainList dl = SIMPLERPC__DOMAIN_INFO__INIT;
   ...
 }
 

src/test/test-full.proto

@@ -19,7 +19,17 @@ enum TestEnum {
   VALUE2097152 = 2097152;
   VALUE268435455 = 268435455;
   VALUE268435456 = 268435456;
-};
+}
+enum TestEnumDupValues {
+  VALUE_A = 42;
+  VALUE_B = 42;
+  VALUE_C = 42;
+  VALUE_D = 666;
+  VALUE_E = 666;
+  VALUE_F = 1000;
+  VALUE_AA = 1000;
+  VALUE_BB = 1001;
+}
 
 message TestFieldNo15 {			// should use 1 byte header
   required string test = 15;

src/test/test-generated-code.c

@@ -3,7 +3,7 @@
 #include <stdlib.h>
 #include <stdio.h>
 
-int main(int argc, char **argv)
+int main()
 {
   Foo__Person person = FOO__PERSON__INIT;
   Foo__Person *person2;

src/test/test-generated-code2.c

@@ -1030,6 +1030,81 @@ static void test_unknown_fields (void)
   foo__empty_mess__free_unpacked (mess2, NULL);
 }
 
+static void
+test_enum_descriptor (const ProtobufCEnumDescriptor *desc)
+{
+  unsigned i;
+  for (i = 0; i < desc->n_values; i++)
+    {
+      const ProtobufCEnumValue *sv = desc->values + i;
+      const ProtobufCEnumValue *vv;
+      const ProtobufCEnumValue *vn;
+      vv = protobuf_c_enum_descriptor_get_value (desc, sv->value);
+      vn = protobuf_c_enum_descriptor_get_value_by_name (desc, sv->name);
+      assert (sv == vv);
+      assert (sv == vn);
+    }
+  for (i = 0; i < desc->n_value_names; i++)
+    {
+      const char *name = desc->values_by_name[i].name;
+      const ProtobufCEnumValue *v;
+      v = protobuf_c_enum_descriptor_get_value_by_name (desc, name);
+      assert (v != NULL);
+    }
+}
+static void
+test_enum_by_name (const ProtobufCEnumDescriptor *desc,
+                   const char *name,
+                   int expected_value)
+{
+  const ProtobufCEnumValue *v;
+  v = protobuf_c_enum_descriptor_get_value_by_name (desc, name);
+  assert (v != NULL);
+  assert (v->value == expected_value);
+}
+
+static void
+test_enum_lookups (void)
+{
+  test_enum_descriptor (&foo__test_enum__descriptor);
+  test_enum_descriptor (&foo__test_enum_small__descriptor);
+  test_enum_descriptor (&foo__test_enum_dup_values__descriptor);
+#define TEST_ENUM_DUP_VALUES(str, shortname) \
+  test_enum_by_name (&foo__test_enum_dup_values__descriptor,  \
+                     str, FOO__TEST_ENUM_DUP_VALUES__##shortname)
+  TEST_ENUM_DUP_VALUES ("VALUE_A", VALUE_A);
+  TEST_ENUM_DUP_VALUES ("VALUE_B", VALUE_B);
+  TEST_ENUM_DUP_VALUES ("VALUE_C", VALUE_C);
+  TEST_ENUM_DUP_VALUES ("VALUE_D", VALUE_D);
+  TEST_ENUM_DUP_VALUES ("VALUE_E", VALUE_E);
+  TEST_ENUM_DUP_VALUES ("VALUE_F", VALUE_F);
+  TEST_ENUM_DUP_VALUES ("VALUE_AA", VALUE_AA);
+  TEST_ENUM_DUP_VALUES ("VALUE_BB", VALUE_BB);
+#undef TEST_ENUM_DUP_VALUES
+}
+
+static void
+test_message_descriptor (const ProtobufCMessageDescriptor *desc)
+{
+  unsigned i;
+  for (i = 0; i < desc->n_fields; i++)
+    {
+      const ProtobufCFieldDescriptor *f = desc->fields + i;
+      const ProtobufCFieldDescriptor *fv;
+      const ProtobufCFieldDescriptor *fn;
+      fv = protobuf_c_message_descriptor_get_field (desc, f->id);
+      fn = protobuf_c_message_descriptor_get_field_by_name (desc, f->name);
+      assert (f == fv);
+      assert (f == fn);
+    }
+}
+static void
+test_message_lookups (void)
+{
+  test_message_descriptor (&foo__test_mess__descriptor);
+  test_message_descriptor (&foo__test_mess_optional__descriptor);
+  test_message_descriptor (&foo__test_mess_required_enum__descriptor);
+}
 
 /* === simple testing framework === */
 
@@ -1105,7 +1180,10 @@ static Test tests[] =
   { "test repeated bytes", test_repeated_bytes },
   { "test repeated SubMess", test_repeated_SubMess },
 
-  { "test unknown fields", test_unknown_fields }
+  { "test unknown fields", test_unknown_fields },
+
+  { "test enum lookups", test_enum_lookups },
+  { "test message lookups", test_message_lookups },
 };
 #define n_tests (sizeof(tests)/sizeof(Test))