graphql: Avoid double serialization of query

Author: leoyvens

graph/src/data/query/query.rs

@@ -112,6 +112,8 @@ pub struct Query {
     pub variables: Option<QueryVariables>,
     pub shape_hash: u64,
     pub network: Option<String>,
+    pub query_text: Arc<String>,
+    pub variables_text: Arc<String>,
     _force_use_of_new: (),
 }
 
@@ -124,23 +126,27 @@ impl Query {
         network: Option<String>,
     ) -> Self {
         let shape_hash = shape_hash(&document);
+
+        let (query_text, variables_text) = if *crate::log::LOG_GQL_TIMING {
+            (
+                document
+                    .format(&graphql_parser::Style::default().indent(0))
+                    .replace('\n', " "),
+                serde_json::to_string(&variables).unwrap_or_default(),
+            )
+        } else {
+            ("(gql logging turned off)".to_owned(), "".to_owned())
+        };
+
         Query {
             schema,
             document,
             variables,
             shape_hash,
             network,
+            query_text: Arc::new(query_text),
+            variables_text: Arc::new(variables_text),
             _force_use_of_new: (),
         }
     }
-
-    pub fn query_text(&self) -> String {
-        self.document
-            .format(&graphql_parser::Style::default().indent(0))
-            .replace('\n', " ")
-    }
-
-    pub fn variables_text(&self) -> String {
-        serde_json::to_string(&self.variables).unwrap_or_default()
-    }
 }

graphql/src/execution/query.rs

@@ -5,7 +5,7 @@ use std::sync::Arc;
 use graph::data::graphql::ext::TypeExt;
 use graph::data::query::{Query as GraphDataQuery, QueryVariables};
 use graph::data::schema::Schema;
-use graph::prelude::QueryExecutionError;
+use graph::prelude::{CheapClone, QueryExecutionError};
 
 use crate::execution::{get_field, get_named_type};
 use crate::introspection::introspection_schema;
@@ -60,13 +60,6 @@ impl Query {
         max_complexity: Option<u64>,
         max_depth: u8,
     ) -> Result<Arc<Self>, Vec<QueryExecutionError>> {
-        let (query_text, variables_text) = match *graph::log::LOG_GQL_TIMING {
-            true => (query.query_text(), query.variables_text()),
-            false => ("(gql logging turned off)".to_owned(), "".to_owned()),
-        };
-        let query_text = Arc::new(query_text);
-        let variables_text = Arc::new(variables_text);
-
         let mut operation = None;
         let mut fragments = HashMap::new();
         for defn in query.document.definitions.into_iter() {
@@ -107,8 +100,8 @@ impl Query {
             shape_hash: query.shape_hash,
             kind,
             network: query.network,
-            query_text,
-            variables_text,
+            query_text: query.query_text.cheap_clone(),
+            variables_text: query.variables_text.cheap_clone(),
             complexity: 0,
         };
 

server/http/src/service.rs

@@ -273,9 +273,8 @@ where
 
         let result = match query {
             Ok(query) => {
-                // Not great that we serialize the query here only for the panic case.
-                let query_text = query.query_text();
-                let variables_text = query.variables_text();
+                let query_text = query.query_text.cheap_clone();
+                let variables_text = query.variables_text.cheap_clone();
 
                 let result =
                     graph::spawn_blocking_allow_panic(service.graphql_runner.run_query(query))
@@ -284,7 +283,7 @@ where
                 match result {
                     Ok(res) => res,
 
-                    // `JoinError` means a panic.
+                    // `Err(JoinError)` means a panic.
                     Err(e) => {
                         let e = e.into_panic();
                         let e = match e

server/index-node/src/service.rs

@@ -119,7 +119,7 @@ where
         let query_result = match result {
             Ok(res) => res,
 
-            // `JoinError` means a panic.
+            // `Err(JoinError)` means a panic.
             Err(e) => {
                 let e = e.into_panic();
                 let e = match e