refactor content sources to avoid evaluating content before serving (vercel/turbo#3334)

fixes WEB-141

This changes the ContentSource API.

Before a ContentSource returned a tuple of specificity and content.
Now it returns a tuple of specificity and a get_content function.

The old way made it very inefficient to combine multiple ContentSource.
All combined ContentSources were ask before, all returned specificity
and content, and they are ordered after that. That required to compute
all contents, even if it is not really used after ordering.

Now we order specificity and a get_content function and only compute the
content (by calling get_content) with the final result.

It also changes how data needs are expressed in the `get_content`
function. Instead of asking it over and over again with more data, it
statically returns the needed data with `vary` and the `get` function is
only called with that data. This is technically more limited than the
old way, but one can workaround that if needed. But practically we
probably never need that. On the other hand it improves the number of
function calls, since the `vary` method is only called once and the
`get` only once per request. Before data needs required at least 2 `get`
calls per request.
This commit is contained in:
Tobias Koppers 2023-01-24 16:16:33 +01:00 committed by GitHub
parent 98c9d723ff
commit b256922644
3 changed files with 29 additions and 32 deletions

View file

@ -77,7 +77,9 @@ impl ContentSource for DevManifestContentSource {
let file = File::from(manifest_content).with_content_type(APPLICATION_JSON);
Ok(ContentSourceResultVc::exact(
ContentSourceContent::Static(AssetContentVc::from(file).into()).cell(),
ContentSourceContent::Static(AssetContentVc::from(file).into())
.cell()
.into(),
))
}
}

View file

@ -1,12 +1,12 @@
use std::collections::HashSet;
use std::collections::BTreeSet;
use anyhow::Result;
use turbo_tasks::{primitives::StringVc, Value};
use turbopack_core::introspect::{Introspectable, IntrospectableVc};
use turbopack_dev_server::source::{
query::QueryValue, ContentSource, ContentSourceContent, ContentSourceData,
ContentSourceDataFilter, ContentSourceDataVary, ContentSourceResultVc, ContentSourceVc,
NeededData, ProxyResult,
ContentSourceDataFilter, ContentSourceDataVary, ContentSourceResult, ContentSourceResultVc,
ContentSourceVc, NeededData, ProxyResult,
};
/// Serves, resizes, optimizes, and re-encodes images to be used with
@ -42,20 +42,17 @@ impl ContentSource for NextImageContentSource {
]
.iter()
.cloned()
.collect::<HashSet<_>>();
.collect::<BTreeSet<_>>();
return Ok(ContentSourceResultVc::exact(
ContentSourceContent::NeedData(NeededData {
source: self_vc.into(),
path: path.to_string(),
vary: ContentSourceDataVary {
url: true,
query: Some(ContentSourceDataFilter::Subset(queries)),
..Default::default()
},
})
.cell(),
));
return Ok(ContentSourceResultVc::need_data(Value::new(NeededData {
source: self_vc.into(),
path: path.to_string(),
vary: ContentSourceDataVary {
url: true,
query: Some(ContentSourceDataFilter::Subset(queries)),
..Default::default()
},
})));
}
Some(query) => query,
};
@ -69,9 +66,8 @@ impl ContentSource for NextImageContentSource {
// formats.
if let Some(path) = url.strip_prefix('/') {
let asset = this.asset_source.get(path, Default::default());
// THERE'S A HUGE PERFORMANCE ISSUE IF THIS MISSES
let inner = asset.await?;
if matches!(&*inner.content.await?, ContentSourceContent::Static(..)) {
if let ContentSourceResult::Result { .. } = &*inner {
return Ok(asset);
}
}
@ -86,7 +82,8 @@ impl ContentSource for NextImageContentSource {
}
.cell(),
)
.cell(),
.cell()
.into(),
))
}
}

View file

@ -86,17 +86,14 @@ impl ContentSource for TurboTasksSource {
let table = viz::table::create_table(tree, tt.stats_type());
viz::table::wrap_html(&table)
} else {
return Ok(ContentSourceResultVc::exact(
ContentSourceContent::NeedData(NeededData {
source: self_vc.into(),
path: path.to_string(),
vary: ContentSourceDataVary {
query: Some(ContentSourceDataFilter::All),
..Default::default()
},
})
.cell(),
));
return Ok(ContentSourceResultVc::need_data(Value::new(NeededData {
source: self_vc.into(),
path: path.to_string(),
vary: ContentSourceDataVary {
query: Some(ContentSourceDataFilter::All),
..Default::default()
},
})));
}
}
"reset" => {
@ -112,7 +109,8 @@ impl ContentSource for TurboTasksSource {
ContentSourceContent::Static(
AssetContentVc::from(File::from(html).with_content_type(TEXT_HTML_UTF_8)).into(),
)
.cell(),
.cell()
.into(),
))
}
}