Skip to content

Commit

Permalink
Throttle calls to package registry for version resolution
Browse files Browse the repository at this point in the history
  The 'HEAD' call that is done to resolve package revisions from
  unpinned versions is already quite cheap, but it would still be better
  to avoid overloading Github with such calls; especially for users of a
  language-server that would compile on-the-fly very often. Upstream
  packages don't change often so there's no need to constantly check the
  etag.

  So we now keep a local version of etags that we fetched, as well as a
  timestamp from the last time we fetched them so that we only re-fetch
  them if more than an hour has elapsed. This should be fairly resilient
  while still massively improving the UX for people showing up after a
  day and trying to use latest 'main' features.

  This means that we now effectively have two caching levels:

  - In the manifest, we store previously fetched etags.
  - In the filesystem, we have a cache of already downloaded zip archives.

  The first cache is basically invalidated every hour, while the second
  cache is only invalidated when a etag changes. For pinned versions,
  nothing is invalidated as they are considered immutable.
  • Loading branch information
KtorZ committed Sep 8, 2023
1 parent 14ecaef commit 6197778
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 41 deletions.
19 changes: 10 additions & 9 deletions crates/aiken-project/src/deps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,10 @@ impl LocalPackages {

pub fn missing_local_packages<'a>(
&self,
manifest: &'a Manifest,
packages: &'a [Package],
root: &PackageName,
) -> Vec<&'a Package> {
manifest
.packages
packages
.iter()
.filter(|p| {
&p.name != root
Expand Down Expand Up @@ -159,14 +158,14 @@ where

let runtime = tokio::runtime::Runtime::new().expect("Unable to start Tokio");

let (manifest, changed) = Manifest::load(event_listener, config, root_path)?;
let (mut manifest, changed) = Manifest::load(event_listener, config, root_path)?;

let local = LocalPackages::load(root_path)?;

local.remove_extra_packages(&manifest, root_path)?;

runtime.block_on(fetch_missing_packages(
&manifest,
&mut manifest,
&local,
project_name,
root_path,
Expand All @@ -183,7 +182,7 @@ where
}

async fn fetch_missing_packages<T>(
manifest: &Manifest,
manifest: &mut Manifest,
local: &LocalPackages,
project_name: PackageName,
root_path: &Path,
Expand All @@ -192,8 +191,10 @@ async fn fetch_missing_packages<T>(
where
T: EventListener,
{
let packages = manifest.packages.to_owned();

let mut missing = local
.missing_local_packages(manifest, &project_name)
.missing_local_packages(&packages, &project_name)
.into_iter()
.peekable();

Expand All @@ -207,7 +208,7 @@ where
let downloader = Downloader::new(root_path);

let statuses = downloader
.download_packages(event_listener, missing, &project_name)
.download_packages(event_listener, missing, &project_name, manifest)
.await?;

let downloaded_from_network = statuses
Expand Down Expand Up @@ -235,5 +236,5 @@ where
}
}

Ok(())
manifest.save(root_path)
}
28 changes: 15 additions & 13 deletions crates/aiken-project/src/deps/downloader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use reqwest::Client;
use zip::result::ZipError;

use crate::{
deps::manifest::Manifest,
error::Error,
package_name::PackageName,
paths::{self, CacheKey},
Expand All @@ -34,28 +35,29 @@ impl<'a> Downloader<'a> {
event_listener: &T,
packages: I,
project_name: &PackageName,
manifest: &mut Manifest,
) -> Result<Vec<(PackageName, bool)>, Error>
where
T: EventListener,
I: Iterator<Item = &'a Package>,
{
future::try_join_all(
packages
.filter(|package| project_name != &package.name)
.map(|package| self.ensure_package_in_build_directory(event_listener, package)),
)
.await
let mut tasks = vec![];

for package in packages.filter(|package| project_name != &package.name) {
let cache_key =
paths::CacheKey::new(&self.http, event_listener, package, manifest).await?;
let task = self.ensure_package_in_build_directory(package, cache_key);
tasks.push(task);
}

future::try_join_all(tasks).await
}

pub async fn ensure_package_in_build_directory<T>(
pub async fn ensure_package_in_build_directory(
&self,
event_listener: &T,
package: &Package,
) -> Result<(PackageName, bool), Error>
where
T: EventListener,
{
let cache_key = paths::CacheKey::new(&self.http, event_listener, package).await?;
cache_key: CacheKey,
) -> Result<(PackageName, bool), Error> {
let downloaded = self
.ensure_package_downloaded(package, &cache_key)
.await
Expand Down
39 changes: 37 additions & 2 deletions crates/aiken-project/src/deps/manifest.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
use std::{fs, path::Path};

use aiken_lang::ast::Span;
use miette::NamedSource;
use serde::{Deserialize, Serialize};
use std::{
collections::BTreeMap,
fs,
path::Path,
time::{Duration, SystemTime},
};

use crate::{
config::{Config, Dependency, Platform},
Expand All @@ -16,6 +20,8 @@ use crate::{
pub struct Manifest {
pub requirements: Vec<Dependency>,
pub packages: Vec<Package>,
#[serde(default)]
pub etags: BTreeMap<String, (SystemTime, String)>,
}

impl Manifest {
Expand Down Expand Up @@ -76,6 +82,34 @@ impl Manifest {

Ok(())
}

pub fn lookup_etag(&self, package: &Package) -> Option<String> {
match self.etags.get(&etag_key(package)) {
None => None,
Some((last_fetched, etag)) => {
let elapsed = SystemTime::now().duration_since(*last_fetched).unwrap();
// Discard any etag older than an hour. So that we throttle call to the package
// registry but we ensure a relatively good synchonization of local packages.
if elapsed > Duration::from_secs(3600) {
None
} else {
Some(etag.clone())
}
}
}
}

pub fn insert_etag(&mut self, package: &Package, etag: String) {
self.etags
.insert(etag_key(package), (SystemTime::now(), etag));
}
}

fn etag_key(package: &Package) -> String {
format!(
"{}/{}@{}",
package.name.owner, package.name.repo, package.version
)
}

#[derive(Deserialize, Serialize, Clone, Debug)]
Expand Down Expand Up @@ -104,6 +138,7 @@ where
})
.collect(),
requirements: config.dependencies.clone(),
etags: BTreeMap::new(),
};

Ok(manifest)
Expand Down
42 changes: 25 additions & 17 deletions crates/aiken-project/src/paths.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::deps::manifest::Package;
use crate::{
deps::manifest::Manifest,
error::Error,
package_name::PackageName,
telemetry::{Event, EventListener},
Expand Down Expand Up @@ -56,6 +57,7 @@ impl CacheKey {
http: &Client,
event_listener: &T,
package: &Package,
manifest: &mut Manifest,
) -> Result<CacheKey, Error>
where
T: EventListener,
Expand All @@ -65,14 +67,26 @@ impl CacheKey {
if is_git_sha_or_tag(&package.version) {
Ok(package.version.to_string())
} else {
match new_cache_key_from_network(http, package).await {
Err(_) => {
event_listener.handle_event(Event::PackageResolveFallback {
name: format!("{}", package.name),
});
new_cache_key_from_cache(package)
}
Ok(cache_key) => Ok(cache_key),
match manifest.lookup_etag(package) {
None => match new_etag_from_network(http, package).await {
Err(_) => {
event_listener.handle_event(Event::PackageResolveFallback {
name: format!("{}", package.name),
});
new_cache_key_from_cache(package)
}
Ok(etag) => {
manifest.insert_etag(package, etag.clone());
Ok(format!(
"{version}@{etag}",
version = package.version.replace('/', "_")
))
}
},
Some(etag) => Ok(format!(
"{version}@{etag}",
version = package.version.replace('/', "_")
)),
}
}?,
))
Expand All @@ -89,7 +103,7 @@ impl CacheKey {
}
}

async fn new_cache_key_from_network(http: &Client, package: &Package) -> Result<String, Error> {
async fn new_etag_from_network(http: &Client, package: &Package) -> Result<String, Error> {
let url = format!(
"https://api.github.com/repos/{}/{}/zipball/{}",
package.name.owner, package.name.repo, package.version
Expand All @@ -104,14 +118,8 @@ async fn new_cache_key_from_network(http: &Client, package: &Package) -> Result<
.get("etag")
.ok_or(Error::UnknownPackageVersion {
package: package.clone(),
})?
.to_str()
.unwrap()
.replace('"', "");
Ok(format!(
"{version}@{etag}",
version = package.version.replace('/', "_")
))
})?;
Ok(etag.to_str().unwrap().replace('"', ""))
}

fn new_cache_key_from_cache(target: &Package) -> Result<String, Error> {
Expand Down
3 changes: 3 additions & 0 deletions examples/hello_world/aiken.lock
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,6 @@ name = "aiken-lang/stdlib"
version = "main"
requirements = []
source = "github"

[etags]
"aiken-lang/stdlib@main" = [{ secs_since_epoch = 1694181032, nanos_since_epoch = 345700000 }, "e9abc03ee3dbf98637e8e2d8b115e1c95573a4168dbbbe517e8499208b67b020"]

0 comments on commit 6197778

Please sign in to comment.