Skip to content

Commit

Permalink
Box cached client callbacks and CachePolicy
Browse files Browse the repository at this point in the history
Looking through the stack trace of `allowed_transitive_url_dependency` with a 800KB stack, i found those two to be the major offenders. These changes make `allowed_transitive_url_dependency` pass with a 800KB stack.
  • Loading branch information
konstin committed Jan 18, 2024
1 parent 5f3fbbb commit a61a2ab
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 7 deletions.
11 changes: 6 additions & 5 deletions crates/puffin-client/src/cached_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,15 @@ enum CachedResponse<Payload: Serialize> {
/// There was no prior cached response or the cache was outdated
///
/// The cache policy is `None` if it isn't storable
ModifiedOrNew(Response, Option<CachePolicy>),
ModifiedOrNew(Response, Option<Box<CachePolicy>>),
}

/// Serialize the actual payload together with its caching information
#[derive(Debug, Deserialize, Serialize)]
pub struct DataWithCachePolicy<Payload: Serialize> {
pub data: Payload,
cache_policy: CachePolicy,
// The cache policy is large (448 bytes at time of writing), reduce the stack size
cache_policy: Box<CachePolicy>,
}

/// Custom caching layer over [`reqwest::Client`] using `http-cache-semantics`.
Expand Down Expand Up @@ -232,14 +233,14 @@ impl CachedClient {
debug!("Found not-modified response for: {url}");
CachedResponse::NotModified(DataWithCachePolicy {
data: cached.data,
cache_policy: new_policy,
cache_policy: Box::new(new_policy),
})
}
AfterResponse::Modified(new_policy, _parts) => {
debug!("Found modified response for: {url}");
CachedResponse::ModifiedOrNew(
res,
new_policy.is_storable().then_some(new_policy),
new_policy.is_storable().then(|| Box::new(new_policy)),
)
}
}
Expand Down Expand Up @@ -272,7 +273,7 @@ impl CachedClient {
CachePolicy::new(&converted_req.into_parts().0, &converted_res.into_parts().0);
Ok(CachedResponse::ModifiedOrNew(
res,
cache_policy.is_storable().then_some(cache_policy),
cache_policy.is_storable().then(|| Box::new(cache_policy)),
))
}
}
3 changes: 2 additions & 1 deletion crates/puffin-client/src/flat_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::collections::btree_map::Entry;
use std::collections::BTreeMap;
use std::path::PathBuf;

use futures::StreamExt;
use futures::{FutureExt, StreamExt};
use reqwest::Response;
use rustc_hash::FxHashMap;
use tracing::{debug, info_span, instrument, warn, Instrument};
Expand Down Expand Up @@ -121,6 +121,7 @@ impl<'a> FlatIndexClient<'a> {
.collect();
Ok(files)
}
.boxed()
.instrument(info_span!("parse_flat_index_html", url = % url))
};
let files = cached_client
Expand Down
4 changes: 3 additions & 1 deletion crates/puffin-client/src/registry_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::str::FromStr;

use async_http_range_reader::{AsyncHttpRangeReader, AsyncHttpRangeReaderError};
use async_zip::tokio::read::seek::ZipFileReader;
use futures::TryStreamExt;
use futures::{FutureExt, TryStreamExt};
use reqwest::{Client, ClientBuilder, Response, StatusCode};
use reqwest_retry::policies::ExponentialBackoff;
use reqwest_retry::RetryTransientMiddleware;
Expand Down Expand Up @@ -206,6 +206,7 @@ impl RegistryClient {
}
}
}
.boxed()
.instrument(info_span!("parse_simple_api", package = %package_name))
};
let result = self
Expand Down Expand Up @@ -335,6 +336,7 @@ impl RegistryClient {
})?;
Ok(metadata)
}
.boxed()
.instrument(info_span!("read_metadata_range_request", wheel = %filename))
};

Expand Down

0 comments on commit a61a2ab

Please sign in to comment.