Change value of channel arg to string form, and reparse in each place we use it.

src/core/ext/client_channel/client_channel.c

@@ -302,11 +302,13 @@ static void on_resolver_result_changed(grpc_exec_ctx *exec_ctx, void *arg,
     channel_arg =
         grpc_channel_args_find(chand->resolver_result, GRPC_ARG_SERVICE_CONFIG);
     if (channel_arg != NULL) {
-      GPR_ASSERT(channel_arg->type == GRPC_ARG_POINTER);
-      grpc_json_tree *service_config_json = channel_arg->value.pointer.p;
-      method_params_table = grpc_method_config_table_create_from_json(
-          service_config_json->root, method_parameters_create_from_json,
+      GPR_ASSERT(channel_arg->type == GRPC_ARG_STRING);
+      grpc_service_config* service_config =
+          grpc_service_config_create(channel_arg->value.string);
+      method_params_table = grpc_service_config_create_method_config_table(
+          service_config, method_parameters_create_from_json,
           &method_parameters_vtable);
+      grpc_service_config_destroy(service_config);
     }
     // Clean up.
     grpc_channel_args_destroy(chand->resolver_result);

src/core/lib/channel/message_size_filter.c

@@ -223,11 +223,13 @@ static void init_channel_elem(grpc_exec_ctx* exec_ctx,
   const grpc_arg* channel_arg =
       grpc_channel_args_find(args->channel_args, GRPC_ARG_SERVICE_CONFIG);
   if (channel_arg != NULL) {
-    GPR_ASSERT(channel_arg->type == GRPC_ARG_POINTER);
-    grpc_json_tree* json_tree = channel_arg->value.pointer.p;
-    chand->method_limit_table = grpc_method_config_table_create_from_json(
-        json_tree->root, message_size_limits_create_from_json,
+    GPR_ASSERT(channel_arg->type == GRPC_ARG_STRING);
+    grpc_service_config* service_config =
+        grpc_service_config_create(channel_arg->value.string);
+    chand->method_limit_table = grpc_service_config_create_method_config_table(
+        service_config, message_size_limits_create_from_json,
         &message_size_limits_vtable);
+    grpc_service_config_destroy(service_config);
   }
 }
 

src/core/lib/json/json.c

@@ -34,8 +34,6 @@
 #include <string.h>
 
 #include <grpc/support/alloc.h>
-#include <grpc/support/string_util.h>
-#include <grpc/support/sync.h>
 
 #include "src/core/lib/json/json.h"
 
@@ -64,60 +62,3 @@ void grpc_json_destroy(grpc_json* json) {
 
   gpr_free(json);
 }
-
-int grpc_json_cmp(const grpc_json* json1, const grpc_json* json2) {
-  if (json1 == NULL) {
-    if (json2 != NULL) return 1;
-    return 0;  // Both NULL.
-  } else {
-    if (json2 == NULL) return -1;
-  }
-  // Compare type.
-  if (json1->type > json2->type) return 1;
-  if (json1->type < json2->type) return -1;
-  // Compare key.
-  if (json1->key == NULL) {
-    if (json2->key != NULL) return -1;
-  } else {
-    if (json2->key == NULL) return 1;
-    int retval = strcmp(json1->key, json2->key);
-    if (retval != 0) return retval;
-  }
-  // Compare value.
-  if (json1->value == NULL) {
-    if (json2->value != NULL) return -1;
-  } else {
-    if (json2->value == NULL) return 1;
-    int retval = strcmp(json1->value, json2->value);
-    if (retval != 0) return retval;
-  }
-  // Recursively compare the next pointer.
-  int retval = grpc_json_cmp(json1->next, json2->next);
-  if (retval != 0) return retval;
-  // Recursively compare the child pointer.
-  retval = grpc_json_cmp(json1->child, json2->child);
-  if (retval != 0) return retval;
-  // Both are the same.
-  return 0;
-}
-
-grpc_json_tree* grpc_json_tree_create(const char* json_string) {
-  grpc_json_tree* tree = gpr_malloc(sizeof(*tree));
-  tree->string = gpr_strdup(json_string);
-  tree->root = grpc_json_parse_string(tree->string);
-  gpr_ref_init(&tree->refs, 1);
-  return tree;
-}
-
-grpc_json_tree* grpc_json_tree_ref(grpc_json_tree* tree) {
-  gpr_ref(&tree->refs);
-  return tree;
-}
-
-void grpc_json_tree_unref(grpc_json_tree* tree) {
-  if (gpr_unref(&tree->refs)) {
-    grpc_json_destroy(tree->root);
-    gpr_free(tree->string);
-    gpr_free(tree);
-  }
-}

src/core/lib/json/json.h

@@ -36,8 +36,6 @@
 
 #include <stdlib.h>
 
-#include <grpc/support/sync.h>
-
 #include "src/core/lib/json/json_common.h"
 
 /* A tree-like structure to hold json values. The key and value pointers
@@ -87,21 +85,4 @@ char* grpc_json_dump_to_string(grpc_json* json, int indent);
 grpc_json* grpc_json_create(grpc_json_type type);
 void grpc_json_destroy(grpc_json* json);
 
-/* Compares two JSON trees. */
-int grpc_json_cmp(const grpc_json* json1, const grpc_json* json2);
-
-/* A wrapper that contains the string used for underlying allocation and
-   is refcounted. */
-typedef struct {
-  grpc_json* root;
-  char* string;
-  gpr_refcount refs;
-} grpc_json_tree;
-
-/* Creates a copy of \a json_string. */
-grpc_json_tree* grpc_json_tree_create(const char* json_string);
-
-grpc_json_tree* grpc_json_tree_ref(grpc_json_tree* tree);
-void grpc_json_tree_unref(grpc_json_tree* tree);
-
 #endif /* GRPC_CORE_LIB_JSON_JSON_H */

src/core/lib/transport/service_config.c

@@ -42,6 +42,40 @@
 #include "src/core/lib/support/string.h"
 #include "src/core/lib/transport/mdstr_hash_table.h"
 
+struct grpc_service_config {
+  char* string;
+  grpc_json* json;
+};
+
+grpc_service_config* grpc_service_config_create(const char* json_string) {
+  grpc_service_config* service_config = gpr_malloc(sizeof(*service_config));
+  service_config->string = gpr_strdup(json_string);
+  service_config->json = grpc_json_parse_string(service_config->string);
+  return service_config;
+}
+
+void grpc_service_config_destroy(grpc_service_config* service_config) {
+  grpc_json_destroy(service_config->json);
+  gpr_free(service_config->string);
+  gpr_free(service_config);
+}
+
+const char* grpc_service_config_get_lb_policy_name(
+    const grpc_service_config* service_config) {
+  const grpc_json* json = service_config->json;
+  if (json->type != GRPC_JSON_OBJECT || json->key != NULL) return NULL;
+  const char* lb_policy_name = NULL;
+  for (grpc_json* field = json->child; field != NULL; field = field->next) {
+    if (field->key == NULL) return NULL;
+    if (strcmp(field->key, "lb_policy_name") == 0) {
+      if (lb_policy_name != NULL) return NULL;  // Duplicate.
+      if (field->type != GRPC_JSON_STRING) return NULL;
+      lb_policy_name = field->value;
+    }
+  }
+  return lb_policy_name;
+}
+
 // Returns the number of names specified in the method config \a json.
 static size_t count_names_in_method_config_json(grpc_json* json) {
   size_t num_names = 0;
@@ -115,10 +149,11 @@ done:
   return retval;
 }
 
-grpc_mdstr_hash_table* grpc_method_config_table_create_from_json(
-    const grpc_json* json,
+grpc_mdstr_hash_table* grpc_service_config_create_method_config_table(
+    const grpc_service_config* service_config,
     void* (*create_value)(const grpc_json* method_config_json),
     const grpc_mdstr_hash_table_vtable* vtable) {
+  const grpc_json* json = service_config->json;
   // Traverse parsed JSON tree.
   if (json->type != GRPC_JSON_OBJECT || json->key != NULL) return NULL;
   size_t num_entries = 0;
@@ -180,42 +215,3 @@ void* grpc_method_config_table_get(const grpc_mdstr_hash_table* table,
   }
   return value;
 }
-
-const char* grpc_service_config_get_lb_policy_name(
-    grpc_json_tree* service_config) {
-  grpc_json* json = service_config->root;
-  if (json->type != GRPC_JSON_OBJECT || json->key != NULL) return NULL;
-  const char* lb_policy_name = NULL;
-  for (grpc_json* field = json->child; field != NULL; field = field->next) {
-    if (field->key == NULL) return NULL;
-    if (strcmp(field->key, "lb_policy_name") == 0) {
-      if (lb_policy_name != NULL) return NULL;  // Duplicate.
-      if (field->type != GRPC_JSON_STRING) return NULL;
-      lb_policy_name = field->value;
-    }
-  }
-  return lb_policy_name;
-}
-
-static void* copy_json_tree(void* t) { return grpc_json_tree_ref(t); }
-
-static void destroy_json_tree(void* t) { grpc_json_tree_unref(t); }
-
-static int cmp_json_tree(void* t1, void* t2) {
-  grpc_json_tree* tree1 = t1;
-  grpc_json_tree* tree2 = t2;
-  return grpc_json_cmp(tree1->root, tree2->root);
-}
-
-static grpc_arg_pointer_vtable service_config_arg_vtable = {
-    copy_json_tree, destroy_json_tree, cmp_json_tree};
-
-grpc_arg grpc_service_config_create_channel_arg(
-    grpc_json_tree* service_config) {
-  grpc_arg arg;
-  arg.type = GRPC_ARG_POINTER;
-  arg.key = GRPC_ARG_SERVICE_CONFIG;
-  arg.value.pointer.p = service_config;
-  arg.value.pointer.vtable = &service_config_arg_vtable;
-  return arg;
-}

src/core/lib/transport/service_config.h

@@ -37,16 +37,29 @@
 #include "src/core/lib/json/json.h"
 #include "src/core/lib/transport/mdstr_hash_table.h"
 
+typedef struct grpc_service_config grpc_service_config;
+
+grpc_service_config* grpc_service_config_create(const char* json_string);
+void grpc_service_config_destroy(grpc_service_config* service_config);
+
+/// Gets the LB policy name from \a service_config.
+/// Returns NULL if no LB policy name was specified.
+/// Caller does NOT take ownership.
+const char* grpc_service_config_get_lb_policy_name(
+    const grpc_service_config* service_config);
+
 /// Creates a method config table based on the data in \a json.
 /// The table's keys are request paths.  The table's value type is
 /// returned by \a create_value(), based on data parsed from the JSON tree.
 /// \a vtable provides methods used to manage the values.
 /// Returns NULL on error.
-grpc_mdstr_hash_table* grpc_method_config_table_create_from_json(
-    const grpc_json* json,
+grpc_mdstr_hash_table* grpc_service_config_create_method_config_table(
+    const grpc_service_config* service_config,
     void* (*create_value)(const grpc_json* method_config_json),
     const grpc_mdstr_hash_table_vtable* vtable);
 
+/// A helper function for looking up values in the table returned by
+/// grpc_service_config_create_method_config_table().
 /// Gets the method config for the specified \a path, which should be of
 /// the form "/service/method".
 /// Returns NULL if the method has no config.
@@ -54,13 +67,4 @@ grpc_mdstr_hash_table* grpc_method_config_table_create_from_json(
 void* grpc_method_config_table_get(const grpc_mdstr_hash_table* table,
                                    const grpc_mdstr* path);
 
-/// Gets the LB policy name from \a service_config.
-/// Returns NULL if no LB policy name was specified.
-/// Caller does NOT take ownership.
-const char* grpc_service_config_get_lb_policy_name(
-    grpc_json_tree* service_config);
-
-/// Creates a channel arg containing \a service_config.
-grpc_arg grpc_service_config_create_channel_arg(grpc_json_tree* service_config);
-
 #endif /* GRPC_CORE_LIB_TRANSPORT_SERVICE_CONFIG_H */

test/core/end2end/connection_refused_test.c

@@ -76,7 +76,10 @@ static void run_test(bool wait_for_ready, bool use_service_config) {
   grpc_channel_args *args = NULL;
   if (use_service_config) {
     GPR_ASSERT(wait_for_ready);
-    grpc_json_tree *service_config_json = grpc_json_tree_create(
+    grpc_arg arg;
+    arg.type = GRPC_ARG_STRING;
+    arg.key = GRPC_ARG_SERVICE_CONFIG;
+    arg.value.string =
         "{\n"
         "  \"method_config\": [ {\n"
         "    \"name\": [\n"
@@ -84,10 +87,8 @@ static void run_test(bool wait_for_ready, bool use_service_config) {
         "    ],\n"
         "    \"wait_for_ready\": true\n"
         "  } ]\n"
-        "}");
-    grpc_arg arg = grpc_service_config_create_channel_arg(service_config_json);
+        "}";
     args = grpc_channel_args_copy_and_add(args, &arg, 1);
-    grpc_json_tree_unref(service_config_json);
   }
 
   /* create a call, channel to a port which will refuse connection */

test/core/end2end/tests/cancel_after_accept.c

@@ -132,7 +132,10 @@ static void test_cancel_after_accept(grpc_end2end_test_config config,
 
   grpc_channel_args *args = NULL;
   if (use_service_config) {
-    grpc_json_tree *service_config_json = grpc_json_tree_create(
+    grpc_arg arg;
+    arg.type = GRPC_ARG_STRING;
+    arg.key = GRPC_ARG_SERVICE_CONFIG;
+    arg.value.string =
         "{\n"
         "  \"method_config\": [ {\n"
         "    \"name\": [\n"
@@ -140,10 +143,8 @@ static void test_cancel_after_accept(grpc_end2end_test_config config,
         "    ],\n"
         "    \"timeout\": { \"seconds\": 5 }\n"
         "  } ]\n"
-        "}");
-    grpc_arg arg = grpc_service_config_create_channel_arg(service_config_json);
+        "}";
     args = grpc_channel_args_copy_and_add(args, &arg, 1);
-    grpc_json_tree_unref(service_config_json);
   }
 
   grpc_end2end_test_fixture f =

test/core/end2end/tests/max_message_length.c

@@ -137,7 +137,10 @@ static void test_max_message_length_on_request(grpc_end2end_test_config config,
   if (use_service_config) {
     // We don't currently support service configs on the server side.
     GPR_ASSERT(send_limit);
-    grpc_json_tree *service_config_json = grpc_json_tree_create(
+    grpc_arg arg;
+    arg.type = GRPC_ARG_STRING;
+    arg.key = GRPC_ARG_SERVICE_CONFIG;
+    arg.value.string =
         "{\n"
         "  \"method_config\": [ {\n"
         "    \"name\": [\n"
@@ -145,10 +148,8 @@ static void test_max_message_length_on_request(grpc_end2end_test_config config,
         "    ],\n"
         "    \"max_request_message_bytes\": 5\n"
         "  } ]\n"
-        "}");
-    grpc_arg arg = grpc_service_config_create_channel_arg(service_config_json);
+        "}";
     client_args = grpc_channel_args_copy_and_add(NULL, &arg, 1);
-    grpc_json_tree_unref(service_config_json);
   } else {
     // Set limit via channel args.
     grpc_arg arg;
@@ -308,7 +309,10 @@ static void test_max_message_length_on_response(grpc_end2end_test_config config,
   if (use_service_config) {
     // We don't currently support service configs on the server side.
     GPR_ASSERT(!send_limit);
-    grpc_json_tree *service_config_json = grpc_json_tree_create(
+    grpc_arg arg;
+    arg.type = GRPC_ARG_STRING;
+    arg.key = GRPC_ARG_SERVICE_CONFIG;
+    arg.value.string =
         "{\n"
         "  \"method_config\": [ {\n"
         "    \"name\": [\n"
@@ -316,10 +320,8 @@ static void test_max_message_length_on_response(grpc_end2end_test_config config,
         "    ],\n"
         "    \"max_response_message_bytes\": 5\n"
         "  } ]\n"
-        "}");
-    grpc_arg arg = grpc_service_config_create_channel_arg(service_config_json);
+        "}";
     client_args = grpc_channel_args_copy_and_add(NULL, &arg, 1);
-    grpc_json_tree_unref(service_config_json);
   } else {
     // Set limit via channel args.
     grpc_arg arg;