Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: asynchronous blob backing store #10969

Merged
merged 33 commits into from
Jul 5, 2021
Merged
Show file tree
Hide file tree
Changes from 31 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
ee27fca
async blob source
jimmywarting Jun 15, 2021
cdf3741
rename byteArrays to parts
jimmywarting Jun 15, 2021
bdcbfc6
fmt + lint
lucacasonato Jun 18, 2021
374aae3
add Rust infrastructure
lucacasonato Jun 18, 2021
6efc469
Adopt to rusts Blob opaque reference
jimmywarting Jun 19, 2021
0f18cb0
Merge branch 'main' into main
lucacasonato Jun 22, 2021
6a282ec
Made size private
jimmywarting Jun 22, 2021
57405b5
Merge branch 'main' into main
lucacasonato Jun 22, 2021
9fd2aa2
lint + fmt
lucacasonato Jun 22, 2021
a73e797
fix compiler error
lucacasonato Jun 22, 2021
5c02715
Solve apply
jimmywarting Jun 22, 2021
2eb5dd7
fix op names
lucacasonato Jun 22, 2021
aba0225
fix tests
lucacasonato Jun 22, 2021
d7db60b
fix blob.slice once more
jimmywarting Jun 22, 2021
23264ea
fix
lucacasonato Jun 22, 2021
aba9e34
format
jimmywarting Jun 22, 2021
2b11283
fix more things
lucacasonato Jun 23, 2021
3649c37
relativeEnd should change with the part.size - not with slice().size
jimmywarting Jun 23, 2021
3e33378
Implement Deno.customInspect for Blob
lucacasonato Jun 24, 2021
0d4978a
fix bad tests
lucacasonato Jun 24, 2021
07d2f8b
Merge remote-tracking branch 'origin/main' into jimmywarting/main
lucacasonato Jun 28, 2021
6972a31
some tests pass
lucacasonato Jun 28, 2021
1fd7d21
update wpt
lucacasonato Jun 30, 2021
8b34b75
What I believe should happen
jimmywarting Jul 2, 2021
6d7fe90
Merge remote-tracking branch 'origin/main' into jimmywarting/main
lucacasonato Jul 3, 2021
e76a01e
gc blobs
lucacasonato Jul 3, 2021
eb7de2d
typo
lucacasonato Jul 4, 2021
b698c7e
remove a comment
lucacasonato Jul 4, 2021
9e5344e
Merge remote-tracking branch 'origin/main' into jimmywarting/main
lucacasonato Jul 4, 2021
e696d50
Merge branch 'main' of github.com:jimmywarting/deno into jimmywarting…
lucacasonato Jul 4, 2021
9db1418
fix timers bench
lucacasonato Jul 4, 2021
20a679a
don't borrow across await point
lucacasonato Jul 5, 2021
2d519e9
remove hyper from deps
lucacasonato Jul 5, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

80 changes: 43 additions & 37 deletions cli/file_fetcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use deno_core::futures;
use deno_core::futures::future::FutureExt;
use deno_core::ModuleSpecifier;
use deno_runtime::deno_fetch::reqwest;
use deno_runtime::deno_web::BlobUrlStore;
use deno_runtime::deno_web::BlobStore;
use deno_runtime::permissions::Permissions;
use log::debug;
use log::info;
Expand Down Expand Up @@ -212,7 +212,7 @@ pub struct FileFetcher {
cache_setting: CacheSetting,
http_cache: HttpCache,
http_client: reqwest::Client,
blob_url_store: BlobUrlStore,
blob_store: BlobStore,
}

impl FileFetcher {
Expand All @@ -221,7 +221,7 @@ impl FileFetcher {
cache_setting: CacheSetting,
allow_remote: bool,
ca_data: Option<Vec<u8>>,
blob_url_store: BlobUrlStore,
blob_store: BlobStore,
) -> Result<Self, AnyError> {
Ok(Self {
auth_tokens: AuthTokens::new(env::var(DENO_AUTH_TOKENS).ok()),
Expand All @@ -230,7 +230,7 @@ impl FileFetcher {
cache_setting,
http_cache,
http_client: create_http_client(get_user_agent(), ca_data)?,
blob_url_store,
blob_store,
})
}

Expand Down Expand Up @@ -360,7 +360,7 @@ impl FileFetcher {
}

/// Get a blob URL.
fn fetch_blob_url(
async fn fetch_blob_url(
&self,
specifier: &ModuleSpecifier,
) -> Result<File, AnyError> {
Expand All @@ -381,20 +381,23 @@ impl FileFetcher {
));
}

let blob_url_storage = self.blob_url_store.borrow();
let blob = blob_url_storage.get(specifier.clone())?.ok_or_else(|| {
custom_error(
"NotFound",
format!("Blob URL not found: \"{}\".", specifier),
)
})?;
let blob_store = self.blob_store.borrow();
let blob =
blob_store
.get_object_url(specifier.clone())?
.ok_or_else(|| {
custom_error(
"NotFound",
format!("Blob URL not found: \"{}\".", specifier),
)
})?;

let content_type = blob.media_type;
let content_type = blob.media_type.clone();
let bytes = blob.read_all().await?;
lucacasonato marked this conversation as resolved.
Show resolved Hide resolved

let (media_type, maybe_charset) =
map_content_type(specifier, Some(content_type.clone()));
let source =
strip_shebang(get_source_from_bytes(blob.data, maybe_charset)?);
let source = strip_shebang(get_source_from_bytes(bytes, maybe_charset)?);

let local =
self
Expand Down Expand Up @@ -525,7 +528,7 @@ impl FileFetcher {
}
result
} else if scheme == "blob" {
let result = self.fetch_blob_url(specifier);
let result = self.fetch_blob_url(specifier).await;
if let Ok(file) = &result {
self.cache.insert(specifier.clone(), file.clone());
}
Expand Down Expand Up @@ -580,6 +583,7 @@ mod tests {
use deno_core::resolve_url;
use deno_core::resolve_url_or_path;
use deno_runtime::deno_web::Blob;
use deno_runtime::deno_web::InMemoryBlobPart;
use std::rc::Rc;
use tempfile::TempDir;

Expand All @@ -588,28 +592,28 @@ mod tests {
maybe_temp_dir: Option<Rc<TempDir>>,
) -> (FileFetcher, Rc<TempDir>) {
let (file_fetcher, temp_dir, _) =
setup_with_blob_url_store(cache_setting, maybe_temp_dir);
setup_with_blob_store(cache_setting, maybe_temp_dir);
(file_fetcher, temp_dir)
}

fn setup_with_blob_url_store(
fn setup_with_blob_store(
cache_setting: CacheSetting,
maybe_temp_dir: Option<Rc<TempDir>>,
) -> (FileFetcher, Rc<TempDir>, BlobUrlStore) {
) -> (FileFetcher, Rc<TempDir>, BlobStore) {
let temp_dir = maybe_temp_dir.unwrap_or_else(|| {
Rc::new(TempDir::new().expect("failed to create temp directory"))
});
let location = temp_dir.path().join("deps");
let blob_url_store = BlobUrlStore::default();
let blob_store = BlobStore::default();
let file_fetcher = FileFetcher::new(
HttpCache::new(&location),
cache_setting,
true,
None,
blob_url_store.clone(),
blob_store.clone(),
)
.expect("setup failed");
(file_fetcher, temp_dir, blob_url_store)
(file_fetcher, temp_dir, blob_store)
}

macro_rules! file_url {
Expand Down Expand Up @@ -948,16 +952,18 @@ mod tests {

#[tokio::test]
async fn test_fetch_blob_url() {
let (file_fetcher, _, blob_url_store) =
setup_with_blob_url_store(CacheSetting::Use, None);
let (file_fetcher, _, blob_store) =
setup_with_blob_store(CacheSetting::Use, None);

let bytes =
"export const a = \"a\";\n\nexport enum A {\n A,\n B,\n C,\n}\n"
.as_bytes()
.to_vec();

let specifier = blob_url_store.insert(
let specifier = blob_store.insert_object_url(
Blob {
data:
"export const a = \"a\";\n\nexport enum A {\n A,\n B,\n C,\n}\n"
.as_bytes()
.to_vec(),
media_type: "application/typescript".to_string(),
parts: vec![Arc::new(Box::new(InMemoryBlobPart::from(bytes)))],
},
None,
);
Expand Down Expand Up @@ -1049,7 +1055,7 @@ mod tests {
CacheSetting::ReloadAll,
true,
None,
BlobUrlStore::default(),
BlobStore::default(),
)
.expect("setup failed");
let result = file_fetcher
Expand All @@ -1076,7 +1082,7 @@ mod tests {
CacheSetting::Use,
true,
None,
BlobUrlStore::default(),
BlobStore::default(),
)
.expect("could not create file fetcher");
let specifier =
Expand Down Expand Up @@ -1104,7 +1110,7 @@ mod tests {
CacheSetting::Use,
true,
None,
BlobUrlStore::default(),
BlobStore::default(),
)
.expect("could not create file fetcher");
let result = file_fetcher_02
Expand Down Expand Up @@ -1265,7 +1271,7 @@ mod tests {
CacheSetting::Use,
true,
None,
BlobUrlStore::default(),
BlobStore::default(),
)
.expect("could not create file fetcher");
let specifier =
Expand Down Expand Up @@ -1296,7 +1302,7 @@ mod tests {
CacheSetting::Use,
true,
None,
BlobUrlStore::default(),
BlobStore::default(),
)
.expect("could not create file fetcher");
let result = file_fetcher_02
Expand Down Expand Up @@ -1406,7 +1412,7 @@ mod tests {
CacheSetting::Use,
false,
None,
BlobUrlStore::default(),
BlobStore::default(),
)
.expect("could not create file fetcher");
let specifier =
Expand All @@ -1433,15 +1439,15 @@ mod tests {
CacheSetting::Only,
true,
None,
BlobUrlStore::default(),
BlobStore::default(),
)
.expect("could not create file fetcher");
let file_fetcher_02 = FileFetcher::new(
HttpCache::new(&location),
CacheSetting::Use,
true,
None,
BlobUrlStore::default(),
BlobStore::default(),
)
.expect("could not create file fetcher");
let specifier =
Expand Down
6 changes: 3 additions & 3 deletions cli/lsp/registries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use deno_core::serde_json::json;
use deno_core::url::Position;
use deno_core::url::Url;
use deno_core::ModuleSpecifier;
use deno_runtime::deno_web::BlobUrlStore;
use deno_runtime::deno_web::BlobStore;
use deno_runtime::permissions::Permissions;
use log::error;
use lspower::lsp;
Expand Down Expand Up @@ -264,7 +264,7 @@ impl Default for ModuleRegistry {
cache_setting,
true,
None,
BlobUrlStore::default(),
BlobStore::default(),
)
.unwrap();

Expand All @@ -283,7 +283,7 @@ impl ModuleRegistry {
CacheSetting::Use,
true,
None,
BlobUrlStore::default(),
BlobStore::default(),
)
.context("Error creating file fetcher in module registry.")
.unwrap();
Expand Down
4 changes: 2 additions & 2 deletions cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ fn create_web_worker_callback(
ts_version: version::TYPESCRIPT.to_string(),
no_color: !colors::use_color(),
get_error_class_fn: Some(&crate::errors::get_error_class_name),
blob_url_store: program_state.blob_url_store.clone(),
blob_store: program_state.blob_store.clone(),
broadcast_channel: program_state.broadcast_channel.clone(),
};

Expand Down Expand Up @@ -207,7 +207,7 @@ pub fn create_main_worker(
.join("location_data")
.join(checksum::gen(&[loc.to_string().as_bytes()]))
}),
blob_url_store: program_state.blob_url_store.clone(),
blob_store: program_state.blob_store.clone(),
broadcast_channel: program_state.broadcast_channel.clone(),
};

Expand Down
10 changes: 5 additions & 5 deletions cli/program_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::source_maps::SourceMapGetter;
use crate::specifier_handler::FetchHandler;
use crate::version;
use deno_runtime::deno_broadcast_channel::InMemoryBroadcastChannel;
use deno_runtime::deno_web::BlobUrlStore;
use deno_runtime::deno_web::BlobStore;
use deno_runtime::inspector_server::InspectorServer;
use deno_runtime::permissions::Permissions;

Expand Down Expand Up @@ -53,7 +53,7 @@ pub struct ProgramState {
pub maybe_import_map: Option<ImportMap>,
pub maybe_inspector_server: Option<Arc<InspectorServer>>,
pub ca_data: Option<Vec<u8>>,
pub blob_url_store: BlobUrlStore,
pub blob_store: BlobStore,
pub broadcast_channel: InMemoryBroadcastChannel,
}

Expand All @@ -79,15 +79,15 @@ impl ProgramState {
CacheSetting::Use
};

let blob_url_store = BlobUrlStore::default();
let blob_store = BlobStore::default();
let broadcast_channel = InMemoryBroadcastChannel::default();

let file_fetcher = FileFetcher::new(
http_cache,
cache_usage,
!flags.no_remote,
ca_data.clone(),
blob_url_store.clone(),
blob_store.clone(),
)?;

let lockfile = if let Some(filename) = &flags.lock {
Expand Down Expand Up @@ -146,7 +146,7 @@ impl ProgramState {
maybe_import_map,
maybe_inspector_server,
ca_data,
blob_url_store,
blob_store,
broadcast_channel,
};
Ok(Arc::new(program_state))
Expand Down
4 changes: 2 additions & 2 deletions cli/specifier_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ pub mod tests {
use crate::file_fetcher::CacheSetting;
use crate::http_cache::HttpCache;
use deno_core::resolve_url_or_path;
use deno_runtime::deno_web::BlobUrlStore;
use deno_runtime::deno_web::BlobStore;
use tempfile::TempDir;

macro_rules! map (
Expand All @@ -599,7 +599,7 @@ pub mod tests {
CacheSetting::Use,
true,
None,
BlobUrlStore::default(),
BlobStore::default(),
)
.expect("could not setup");
let disk_cache = deno_dir.gen_cache;
Expand Down
6 changes: 3 additions & 3 deletions cli/standalone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use deno_core::ModuleLoader;
use deno_core::ModuleSpecifier;
use deno_core::OpState;
use deno_runtime::deno_broadcast_channel::InMemoryBroadcastChannel;
use deno_runtime::deno_web::BlobUrlStore;
use deno_runtime::deno_web::BlobStore;
use deno_runtime::permissions::Permissions;
use deno_runtime::permissions::PermissionsOptions;
use deno_runtime::worker::MainWorker;
Expand Down Expand Up @@ -213,7 +213,7 @@ pub async fn run(
let main_module = resolve_url(SPECIFIER)?;
let program_state = ProgramState::build(flags).await?;
let permissions = Permissions::from_options(&metadata.permissions);
let blob_url_store = BlobUrlStore::default();
let blob_store = BlobStore::default();
let broadcast_channel = InMemoryBroadcastChannel::default();
let module_loader = Rc::new(EmbeddedModuleLoader(source_code));
let create_web_worker_cb = Arc::new(|_| {
Expand Down Expand Up @@ -246,7 +246,7 @@ pub async fn run(
get_error_class_fn: Some(&get_error_class_name),
location: metadata.location,
origin_storage_dir: None,
blob_url_store,
blob_store,
broadcast_channel,
};
let mut worker =
Expand Down
11 changes: 11 additions & 0 deletions cli/tests/blob_gc_finalization.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// This test creates 1024 blobs of 128 MB each. This will only work if the blobs
// and their backing data is GCed as expected.
for (let i = 0; i < 1024; i++) {
// Create a 128MB byte array, and then a blob from it.
const buf = new Uint8Array(128 * 1024 * 1024);
new Blob([buf]);
// It is very important that there is a yield here, otherwise the finalizer
// for the blob is not called and the memory is not freed.
await new Promise((resolve) => setTimeout(resolve, 0));
}
console.log("GCed all blobs");
1 change: 1 addition & 0 deletions cli/tests/blob_gc_finalization.js.out
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
GCed all blobs
Loading