Skip to content

Commit

Permalink
move: source service eagerly hydrates modules in memory (MystenLabs#1…
Browse files Browse the repository at this point in the history
…2840)

## Description 

Eagerly reads and hydrates source code on server start. Doing this
eagerly is reasonable right now since source is small (we might opt for
gzip later too):

```
244K    sui-framework/packages/deepbook/sources
 72K    sui-framework/packages/move-stdlib/sources
348K    sui-framework/packages/sui-framework/sources
240K    sui-framework/packages/sui-system/sources
```

The type is set up with `Option` to allow hydrating source on first
request too, but I didn't want to go that route because then we need to
implemented a writer lock and seems pretty overkill given scope right
now.


---

For fun I looked at the local latency difference before/after this PR.
This isn't really meaningful on a local setup, but I've got the tooling
set up with `vegeta` for testing eventual deployment so figured to look
anyway.

100 requests per second over 15 seconds (latencies generally lower with
this PR).

Before

```
Requests      [total, rate, throughput]  1500, 100.07, 100.06
Duration      [total, attack, wait]      14.991483416s, 14.989767291s, 1.716125ms
Latencies     [mean, 50, 95, 99, max]    1.544413ms, 1.475097ms, 2.169692ms, 3.43996ms, 8.906125ms
Bytes In      [total, mean]              5073000, 3382.00
```

After

```
Requests      [total, rate, throughput]  1500, 100.07, 100.06
Duration      [total, attack, wait]      14.991182792s, 14.990031292s, 1.1515ms
Latencies     [mean, 50, 95, 99, max]    1.316599ms, 1.270261ms, 1.769853ms, 2.501024ms, 5.567708ms
Bytes In      [total, mean]              5073000, 3382.00
```

## Test Plan 

Updated test to use hydrated value.
  • Loading branch information
rvantonder authored Jul 5, 2023
1 parent 04092e7 commit 7c82907
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 22 deletions.
32 changes: 21 additions & 11 deletions crates/sui-source-validation-service/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,15 @@ pub struct Packages {
pub paths: Vec<String>,
}

/// Map (package address, module name) tuples to verified source paths.
type SourceLookup = BTreeMap<(AccountAddress, Symbol), PathBuf>;
#[derive(Debug)]
pub struct SourceInfo {
pub path: PathBuf,
// Is Some when content is hydrated from disk.
pub source: Option<String>,
}

/// Map (package address, module name) tuples to verified source info.
type SourceLookup = BTreeMap<(AccountAddress, Symbol), SourceInfo>;

pub async fn verify_package(
client: &SuiClient,
Expand Down Expand Up @@ -73,10 +80,14 @@ pub async fn verify_package(
for v in &compiled_package.package.root_compiled_units {
match v.unit {
CompiledUnitEnum::Module(ref m) => {
map.insert((address, m.name), v.source_path.to_path_buf())
let path = v.source_path.to_path_buf();
let source = Some(fs::read_to_string(path.as_path())?);
map.insert((address, m.name), SourceInfo { path, source })
}
CompiledUnitEnum::Script(ref m) => {
map.insert((address, m.name), v.source_path.to_path_buf())
let path = v.source_path.to_path_buf();
let source = Some(fs::read_to_string(path.as_path())?);
map.insert((address, m.name), SourceInfo { path, source })
}
};
}
Expand Down Expand Up @@ -264,16 +275,15 @@ async fn api_route(
let error = format!("Invalid hex address {address}");
return (StatusCode::BAD_REQUEST, Json(ErrorResponse { error }).into_response())
};
let Some(path) = app_state.sources.get(&(address, symbol)) else {
let Some(SourceInfo {source : Some(source), ..}) = app_state.sources.get(&(address, symbol)) else {
let error = format!("No source found for {symbol} at address {address}" );
return (StatusCode::BAD_REQUEST, Json(ErrorResponse { error }).into_response())
};
let Ok(source) = fs::read_to_string(path) else {
let error = "Error reading source from disk".to_string();
return (StatusCode::INTERNAL_SERVER_ERROR, Json(ErrorResponse { error }).into_response())
return (StatusCode::NOT_FOUND, Json(ErrorResponse { error }).into_response())
};
(
StatusCode::OK,
Json(SourceResponse { source }).into_response(),
Json(SourceResponse {
source: source.to_owned(),
})
.into_response(),
)
}
18 changes: 7 additions & 11 deletions crates/sui-source-validation-service/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ use std::{collections::BTreeMap, path::PathBuf};
use move_core_types::account_address::AccountAddress;
use move_symbol_pool::Symbol;
use sui_source_validation_service::{
initialize, serve, verify_packages, AppState, CloneCommand, Config, Packages, SourceResponse,
initialize, serve, verify_packages, AppState, CloneCommand, Config, Packages, SourceInfo,
SourceResponse,
};
use test_cluster::TestClusterBuilder;

Expand Down Expand Up @@ -80,7 +81,10 @@ async fn test_api_route() -> anyhow::Result<()> {
AccountAddress::from_hex_literal(address).unwrap(),
Symbol::from(module),
),
source_path,
SourceInfo {
path: source_path,
source: Some("module address {...}".to_owned()),
},
);
tokio::spawn(serve(AppState { sources }).expect("Cannot start service."));

Expand All @@ -96,15 +100,7 @@ async fn test_api_route() -> anyhow::Result<()> {
.json::<SourceResponse>()
.await?;

let expected = expect![
r#"
// Copyright (c) Mysten Labs, Inc.
// SPDX-License-Identifier: Apache-2.0
/// Dummy module for testing
module std::address {}
"#
];
let expected = expect!["module address {...}"];
expected.assert_eq(&json.source);
Ok(())
}
Expand Down

0 comments on commit 7c82907

Please sign in to comment.