Skip to content

Commit

Permalink
fix: improve performance for EnvIO (#1574)
Browse files Browse the repository at this point in the history
Co-authored-by: Tushar Mathur <[email protected]>
  • Loading branch information
meskill and tusharmath authored Mar 28, 2024
1 parent 7de18c1 commit 92af027
Show file tree
Hide file tree
Showing 15 changed files with 168 additions and 160 deletions.
259 changes: 130 additions & 129 deletions Cargo.lock

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions aws-lambda/src/runtime.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::borrow::Cow;
use std::sync::Arc;

use anyhow::anyhow;
Expand All @@ -12,10 +13,10 @@ use crate::http::init_http;
pub struct LambdaEnv;

impl EnvIO for LambdaEnv {
fn get(&self, key: &str) -> Option<String> {
fn get(&self, key: &str) -> Option<Cow<'_, str>> {
// AWS Lambda sets environment variables
// as real env vars in the runtime.
std::env::var(key).ok()
std::env::var(key).ok().map(Cow::from)
}
}

Expand Down
3 changes: 2 additions & 1 deletion benches/data_loader_bench.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::borrow::Cow;
use std::collections::BTreeSet;
use std::num::NonZeroU64;
use std::sync::atomic::{AtomicUsize, Ordering};
Expand Down Expand Up @@ -30,7 +31,7 @@ impl HttpIO for MockHttpClient {
struct Env {}
#[async_trait]
impl EnvIO for Env {
fn get(&self, _: &str) -> Option<String> {
fn get(&self, _: &str) -> Option<Cow<'_, str>> {
unimplemented!("Not needed for this bench")
}
}
Expand Down
2 changes: 1 addition & 1 deletion benches/impl_path_string_for_evaluation_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ impl HttpIO for Http {
struct Env {}

impl EnvIO for Env {
fn get(&self, _: &str) -> Option<String> {
fn get(&self, _: &str) -> Option<Cow<'_, str>> {
unimplemented!("Not needed for this bench")
}
}
Expand Down
5 changes: 3 additions & 2 deletions cloudflare/src/env.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::borrow::Cow;
use std::rc::Rc;

use tailcall::EnvIO;
Expand All @@ -11,8 +12,8 @@ unsafe impl Send for CloudflareEnv {}
unsafe impl Sync for CloudflareEnv {}

impl EnvIO for CloudflareEnv {
fn get(&self, key: &str) -> Option<String> {
self.env.var(key).ok().map(|s| s.to_string())
fn get(&self, key: &str) -> Option<Cow<'_, str>> {
self.env.var(key).ok().map(|v| Cow::from(v.to_string()))
}
}

Expand Down
6 changes: 2 additions & 4 deletions cloudflare/src/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,8 @@ pub struct CloudflareFileIO {
}

impl CloudflareFileIO {
pub fn init(env: Rc<Env>, bucket_id: String) -> anyhow::Result<Self> {
let bucket = env
.bucket(bucket_id.as_str())
.map_err(|e| anyhow!(e.to_string()))?;
pub fn init(env: Rc<Env>, bucket_id: &str) -> anyhow::Result<Self> {
let bucket = env.bucket(bucket_id).map_err(|e| anyhow!(e.to_string()))?;
let bucket = Rc::new(bucket);
Ok(CloudflareFileIO { bucket })
}
Expand Down
4 changes: 2 additions & 2 deletions cloudflare/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ fn init_env(env: Rc<worker::Env>) -> Arc<dyn EnvIO> {
Arc::new(env::CloudflareEnv::init(env))
}

fn init_file(env: Rc<worker::Env>, bucket_id: String) -> anyhow::Result<Arc<dyn FileIO>> {
fn init_file(env: Rc<worker::Env>, bucket_id: &str) -> anyhow::Result<Arc<dyn FileIO>> {
Ok(Arc::new(file::CloudflareFileIO::init(env, bucket_id)?))
}

Expand All @@ -35,7 +35,7 @@ pub fn init(env: Rc<worker::Env>) -> anyhow::Result<TargetRuntime> {
http: http.clone(),
http2_only: http.clone(),
env: init_env(env.clone()),
file: init_file(env.clone(), bucket_id)?,
file: init_file(env.clone(), &bucket_id)?,
cache: init_cache(env),
extensions: Arc::new(vec![]),
})
Expand Down
7 changes: 4 additions & 3 deletions src/cli/runtime/env.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::borrow::Cow;
use std::collections::HashMap;

use crate::EnvIO;
Expand All @@ -8,8 +9,8 @@ pub struct EnvNative {
}

impl EnvIO for EnvNative {
fn get(&self, key: &str) -> Option<String> {
self.vars.get(key).cloned()
fn get(&self, key: &str) -> Option<Cow<'_, str>> {
self.vars.get(key).map(Cow::from)
}
}

Expand All @@ -35,7 +36,7 @@ mod tests {
vars.insert("EXISTING_VAR".to_string(), "value".to_string());
let test_env = EnvNative { vars };
let result = test_env.get("EXISTING_VAR");
assert_eq!(result, Some("value".to_string()));
assert_eq!(result, Some("value".into()));
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion src/config/reader_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ impl<'a> PathString for ConfigReaderContext<'a> {
path.split_first()
.and_then(|(head, tail)| match head.as_ref() {
"vars" => self.vars.get(tail[0].as_ref()).map(|v| v.into()),
"env" => self.env.get(tail[0].as_ref()).map(|v| v.into()),
"env" => self.env.get(tail[0].as_ref()),
_ => None,
})
}
Expand Down
2 changes: 1 addition & 1 deletion src/lambda/evaluation_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ impl<'a, Ctx: ResolverContextLike<'a>> EvaluationContext<'a, Ctx> {
value.to_str().ok()
}

pub fn env_var(&self, key: &str) -> Option<String> {
pub fn env_var(&self, key: &str) -> Option<Cow<'_, str>> {
self.request_ctx.runtime.env.get(key)
}

Expand Down
7 changes: 4 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,15 @@ pub mod tracing;
pub mod try_fold;
pub mod valid;

use std::borrow::Cow;
use std::hash::Hash;
use std::num::NonZeroU64;

use async_graphql_value::ConstValue;
use http::Response;

pub trait EnvIO: Send + Sync + 'static {
fn get(&self, key: &str) -> Option<String>;
fn get(&self, key: &str) -> Option<Cow<'_, str>>;
}

#[async_trait::async_trait]
Expand Down Expand Up @@ -94,8 +95,8 @@ pub mod tests {
pub struct TestEnvIO(HashMap<String, String>);

impl EnvIO for TestEnvIO {
fn get(&self, key: &str) -> Option<String> {
self.0.get(key).cloned()
fn get(&self, key: &str) -> Option<Cow<'_, str>> {
self.0.get(key).map(Cow::from)
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl<'a, Ctx: ResolverContextLike<'a>> PathString for EvaluationContext<'a, Ctx>
"args" => convert_value(ctx.path_arg(tail)?),
"headers" => ctx.header(tail[0].as_ref()).map(|v| v.into()),
"vars" => ctx.var(tail[0].as_ref()).map(|v| v.into()),
"env" => ctx.env_var(tail[0].as_ref()).map(|v| v.into()),
"env" => ctx.env_var(tail[0].as_ref()),
_ => None,
})
}
Expand Down Expand Up @@ -123,8 +123,8 @@ mod tests {
}

impl EnvIO for Env {
fn get(&self, key: &str) -> Option<String> {
self.env.get(key).cloned()
fn get(&self, key: &str) -> Option<Cow<'_, str>> {
self.env.get(key).map(Cow::from)
}
}

Expand Down
5 changes: 3 additions & 2 deletions src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ impl TargetRuntime {

#[cfg(test)]
pub mod test {
use std::borrow::Cow;
use std::collections::HashMap;
use std::sync::Arc;
use std::time::Duration;
Expand Down Expand Up @@ -152,8 +153,8 @@ pub mod test {
}

impl EnvIO for TestEnvIO {
fn get(&self, key: &str) -> Option<String> {
self.vars.get(key).cloned()
fn get(&self, key: &str) -> Option<Cow<'_, str>> {
self.vars.get(key).map(Cow::from)
}
}

Expand Down
10 changes: 6 additions & 4 deletions tests/execution_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ extern crate core;

mod telemetry;

use std::borrow::Cow;
use std::collections::{BTreeMap, HashMap};
use std::path::{Path, PathBuf};
use std::str::FromStr;
Expand Down Expand Up @@ -38,6 +39,7 @@ use url::Url;

#[cfg(test)]
pub mod test {
use std::borrow::Cow;
use std::collections::HashMap;
use std::sync::Arc;
use std::time::Duration;
Expand Down Expand Up @@ -154,8 +156,8 @@ pub mod test {
}

impl EnvIO for TestEnvIO {
fn get(&self, key: &str) -> Option<String> {
self.vars.get(key).cloned()
fn get(&self, key: &str) -> Option<Cow<'_, str>> {
self.vars.get(key).map(Cow::from)
}
}

Expand Down Expand Up @@ -238,8 +240,8 @@ pub struct Env {
}

impl EnvIO for Env {
fn get(&self, key: &str) -> Option<String> {
self.env.get(key).cloned()
fn get(&self, key: &str) -> Option<Cow<'_, str>> {
self.env.get(key).map(Cow::from)
}
}

Expand Down
5 changes: 3 additions & 2 deletions tests/server_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use tailcall::{blueprint, EnvIO, FileIO, HttpIO};

#[cfg(test)]
pub mod test {
use std::borrow::Cow;
use std::collections::HashMap;
use std::sync::Arc;
use std::time::Duration;
Expand Down Expand Up @@ -118,8 +119,8 @@ pub mod test {
}

impl EnvIO for TestEnvIO {
fn get(&self, key: &str) -> Option<String> {
self.vars.get(key).cloned()
fn get(&self, key: &str) -> Option<Cow<'_, str>> {
self.vars.get(key).map(Cow::from)
}
}

Expand Down

0 comments on commit 92af027

Please sign in to comment.