Skip to content

Commit

Permalink
Work around compiler bug
Browse files Browse the repository at this point in the history
This tweaks the Agora context type we use in order to avoid a recurring
Rust compiler bug (rust-lang/rust#71723) which
makes it suddenly impossible to carry the context across an await. This
comes at the cost of increasing String allocations. Though the use of
snmalloc mitigates this cost somewhat.
  • Loading branch information
Theodus committed Apr 13, 2023
1 parent 4aca9fa commit 7f30957
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 18 deletions.
32 changes: 16 additions & 16 deletions graph-gateway/src/block_constraints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ pub fn block_constraints<'c>(context: &'c Context<'c>) -> Option<BTreeSet<BlockC
let defaults = query
.variable_definitions
.iter()
.filter(|d| !vars.0.contains_key(d.name))
.filter_map(|d| Some((d.name, d.default_value.as_ref()?.to_graphql())))
.collect::<BTreeMap<&str, StaticValue>>();
.filter(|d| !vars.0.contains_key(&d.name))
.filter_map(|d| Some((d.name.clone(), d.default_value.as_ref()?.to_graphql())))
.collect::<BTreeMap<String, StaticValue>>();
(&query.selection_set, defaults)
}
OperationDefinition::Query(_)
Expand Down Expand Up @@ -88,9 +88,9 @@ pub fn make_query_deterministic(
let defaults = query
.variable_definitions
.iter()
.filter(|d| !vars.0.contains_key(d.name))
.filter_map(|d| Some((d.name, d.default_value.as_ref()?.to_graphql())))
.collect::<BTreeMap<&str, StaticValue>>();
.filter(|d| !vars.0.contains_key(&d.name))
.filter_map(|d| Some((d.name.clone(), d.default_value.as_ref()?.to_graphql())))
.collect::<BTreeMap<String, StaticValue>>();
(&mut query.selection_set, defaults)
}
OperationDefinition::Query(_)
Expand Down Expand Up @@ -123,7 +123,7 @@ pub fn make_query_deterministic(
None => {
selection_field
.arguments
.push(("block", deterministic_block(&latest.hash)));
.push(("block".to_string(), deterministic_block(&latest.hash)));
}
};
}
Expand All @@ -146,21 +146,21 @@ pub fn make_query_deterministic(
.ok()
}

fn deterministic_block<'c>(hash: &Bytes32) -> Value<'c, &'c str> {
fn deterministic_block<'c>(hash: &Bytes32) -> Value<'c, String> {
Value::Object(BTreeMap::from_iter([(
"hash",
"hash".to_string(),
Value::String(hash.to_string()),
)]))
}

fn field_constraint<'c>(
fn field_constraint(
vars: &QueryVariables,
defaults: &BTreeMap<&str, StaticValue>,
field: &Value<'c, &'c str>,
defaults: &BTreeMap<String, StaticValue>,
field: &Value<'_, String>,
) -> Option<BlockConstraint> {
match field {
Value::Object(fields) => parse_constraint(vars, defaults, fields),
Value::Variable(name) => match vars.get(*name)? {
Value::Variable(name) => match vars.get(name)? {
Value::Object(fields) => parse_constraint(vars, defaults, fields),
_ => None,
},
Expand All @@ -170,7 +170,7 @@ fn field_constraint<'c>(

fn parse_constraint<'c, T: Text<'c>>(
vars: &QueryVariables,
defaults: &BTreeMap<&str, StaticValue>,
defaults: &BTreeMap<String, StaticValue>,
fields: &BTreeMap<T::Value, Value<'c, T>>,
) -> Option<BlockConstraint> {
let field = fields.iter().at_most_one().ok()?;
Expand All @@ -190,7 +190,7 @@ fn parse_constraint<'c, T: Text<'c>>(
fn parse_hash<'c, T: Text<'c>>(
hash: &Value<'c, T>,
variables: &QueryVariables,
defaults: &BTreeMap<&str, StaticValue>,
defaults: &BTreeMap<String, StaticValue>,
) -> Option<Bytes32> {
match hash {
Value::String(hash) => hash.parse().ok(),
Expand All @@ -208,7 +208,7 @@ fn parse_hash<'c, T: Text<'c>>(
fn parse_number<'c, T: Text<'c>>(
number: &Value<'c, T>,
variables: &QueryVariables,
defaults: &BTreeMap<&str, StaticValue>,
defaults: &BTreeMap<String, StaticValue>,
) -> Option<u64> {
let n = match number {
Value::Int(n) => n,
Expand Down
2 changes: 1 addition & 1 deletion graph-gateway/src/client_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,6 @@ fn count_top_level_selection_sets(ctx: &AgoraContext) -> anyhow::Result<usize> {
OperationDefinition::Mutation(_) => bail!("Mutations not yet supported"),
OperationDefinition::Subscription(_) => bail!("Subscriptions not yet supported"),
})
.collect::<anyhow::Result<Vec<&SelectionSet<&str>>>>()?;
.collect::<anyhow::Result<Vec<&SelectionSet<String>>>>()?;
Ok(selection_sets.into_iter().map(|set| set.items.len()).sum())
}
4 changes: 3 additions & 1 deletion indexer-selection/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ use std::{
sync::Arc,
};

pub type Context<'c> = cost_model::Context<'c, &'c str>;
// We have to use `String` instead of `&'c str` here because of compiler bug triggered when holding
// a context across an await. See https://github.com/rust-lang/rust/issues/71723
pub type Context<'c> = cost_model::Context<'c, String>;

#[derive(Clone, Debug)]
pub struct Selection {
Expand Down

0 comments on commit 7f30957

Please sign in to comment.