Skip to content

Worked on vector-related performance #692

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

Merged
merged 6 commits into from
May 31, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 13 additions & 8 deletions src/db/add_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,14 +185,19 @@ fn initialize_package_in_database(conn: &Connection, pkg: &MetadataPackage) -> R

/// Convert dependencies into Vec<(String, String, String)>
fn convert_dependencies(pkg: &MetadataPackage) -> Vec<(String, String, String)> {
let mut dependencies: Vec<(String, String, String)> = Vec::new();
for dependency in &pkg.dependencies {
let name = dependency.name.clone();
let version = dependency.req.clone();
let kind = dependency.kind.clone().unwrap_or_else(|| "normal".into());
dependencies.push((name, version, kind.to_string()));
}
dependencies
pkg.dependencies
.iter()
.map(|dependency| {
let name = dependency.name.clone();
let version = dependency.req.clone();
let kind = dependency
.kind
.clone()
.unwrap_or_else(|| "normal".to_string());

(name, version, kind)
})
.collect()
}

/// Reads readme if there is any read defined in Cargo.toml of a Package
Expand Down
6 changes: 3 additions & 3 deletions src/db/blacklist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,11 @@ mod tests {
crate::test::wrapper(|env| {
let db = env.db();

assert!(is_blacklisted(&db.conn(), "crate foo")? == false);
assert!(!is_blacklisted(&db.conn(), "crate foo")?);
add_crate(&db.conn(), "crate foo")?;
assert!(is_blacklisted(&db.conn(), "crate foo")? == true);
assert!(is_blacklisted(&db.conn(), "crate foo")?);
remove_crate(&db.conn(), "crate foo")?;
assert!(is_blacklisted(&db.conn(), "crate foo")? == false);
assert!(!is_blacklisted(&db.conn(), "crate foo")?);
Ok(())
});
}
Expand Down
24 changes: 11 additions & 13 deletions src/db/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,15 @@ pub fn add_path_into_database<P: AsRef<Path>>(
}

fn file_list_to_json(file_list: Vec<(PathBuf, String)>) -> Result<Value> {
let mut file_list_json: Vec<Value> = Vec::new();

for file in file_list {
let mut v = Vec::with_capacity(2);
v.push(Value::String(file.1));
v.push(Value::String(
file.0.into_os_string().into_string().unwrap(),
));

file_list_json.push(Value::Array(v));
}

Ok(Value::Array(file_list_json))
let file_list: Vec<_> = file_list
.into_iter()
.map(|(path, name)| {
Value::Array(vec![
Value::String(name),
Value::String(path.into_os_string().into_string().unwrap()),
])
})
.collect();

Ok(Value::Array(file_list))
}
40 changes: 21 additions & 19 deletions src/docbuilder/limits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ fn scale(value: usize, interval: usize, labels: &[&str]) -> String {
mod test {
use super::*;
use crate::test::*;

#[test]
fn retrieve_limits() {
wrapper(|env| {
Expand Down Expand Up @@ -151,40 +152,41 @@ mod test {
targets: 1,
..Limits::default()
};
db.conn().query("INSERT INTO sandbox_overrides (crate_name, max_memory_bytes, timeout_seconds, max_targets)
VALUES ($1, $2, $3, $4)",
&[&krate, &(limits.memory as i64), &(limits.timeout.as_secs() as i32), &(limits.targets as i32)])?;
db.conn().query(
"INSERT INTO sandbox_overrides (crate_name, max_memory_bytes, timeout_seconds, max_targets)
VALUES ($1, $2, $3, $4)",
&[&krate, &(limits.memory as i64), &(limits.timeout.as_secs() as i32), &(limits.targets as i32)]
)?;
assert_eq!(limits, Limits::for_crate(&db.conn(), krate)?);
Ok(())
});
}

#[test]
fn display_limits() {
let limits = Limits {
memory: 102400,
memory: 102_400,
timeout: Duration::from_secs(300),
targets: 1,
..Limits::default()
};
let display = limits.for_website();
assert_eq!(display.get("Network access"), Some(&"blocked".into()));
assert_eq!(
display.get("Network access".into()),
Some(&"blocked".into())
);
assert_eq!(
display.get("Maximum size of a build log".into()),
display.get("Maximum size of a build log"),
Some(&"100 KB".into())
);
assert_eq!(
display.get("Maximum number of build targets".into()),
display.get("Maximum number of build targets"),
Some(&limits.targets.to_string())
);
assert_eq!(
display.get("Maximum rustdoc execution time".into()),
display.get("Maximum rustdoc execution time"),
Some(&"5 minutes".into())
);
assert_eq!(display.get("Available RAM".into()), Some(&"100 KB".into()));
assert_eq!(display.get("Available RAM"), Some(&"100 KB".into()));
}

#[test]
fn scale_limits() {
// time
Expand All @@ -197,18 +199,18 @@ mod test {
assert_eq!(SIZE_SCALE(100), "100 bytes");
assert_eq!(SIZE_SCALE(1024), "1 KB");
assert_eq!(SIZE_SCALE(10240), "10 KB");
assert_eq!(SIZE_SCALE(1048576), "1 MB");
assert_eq!(SIZE_SCALE(10485760), "10 MB");
assert_eq!(SIZE_SCALE(1073741824), "1 GB");
assert_eq!(SIZE_SCALE(10737418240), "10 GB");
assert_eq!(SIZE_SCALE(1_048_576), "1 MB");
assert_eq!(SIZE_SCALE(10_485_760), "10 MB");
assert_eq!(SIZE_SCALE(1_073_741_824), "1 GB");
assert_eq!(SIZE_SCALE(10_737_418_240), "10 GB");
assert_eq!(SIZE_SCALE(std::u32::MAX as usize), "4 GB");

// fractional sizes
assert_eq!(TIME_SCALE(90), "1.5 minutes");
assert_eq!(TIME_SCALE(5400), "1.5 hours");

assert_eq!(SIZE_SCALE(1288490189), "1.2 GB");
assert_eq!(SIZE_SCALE(3758096384), "3.5 GB");
assert_eq!(SIZE_SCALE(1048051712), "999.5 MB");
assert_eq!(SIZE_SCALE(1_288_490_189), "1.2 GB");
assert_eq!(SIZE_SCALE(3_758_096_384), "3.5 GB");
assert_eq!(SIZE_SCALE(1_048_051_712), "999.5 MB");
}
}
4 changes: 2 additions & 2 deletions src/docbuilder/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,8 @@ mod test {
let metadata = Metadata::from_str(manifest);

assert!(metadata.features.is_some());
assert!(metadata.all_features == true);
assert!(metadata.no_default_features == true);
assert!(metadata.all_features);
assert!(metadata.no_default_features);
assert!(metadata.default_target.is_some());
assert!(metadata.rustdoc_args.is_some());

Expand Down
74 changes: 35 additions & 39 deletions src/index/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,47 +116,43 @@ fn get_owners(pkg: &MetadataPackage) -> Result<Vec<CrateOwner>> {
res.read_to_string(&mut body).unwrap();
let json: Value = serde_json::from_str(&body[..])?;

let mut result = Vec::new();
if let Some(owners) = json
let owners = json
.as_object()
.and_then(|j| j.get("users"))
.and_then(|j| j.as_array())
{
for owner in owners {
// FIXME: I know there is a better way to do this
let avatar = owner
.as_object()
.and_then(|o| o.get("avatar"))
.and_then(|o| o.as_str())
.unwrap_or("");
let email = owner
.as_object()
.and_then(|o| o.get("email"))
.and_then(|o| o.as_str())
.unwrap_or("");
let login = owner
.as_object()
.and_then(|o| o.get("login"))
.and_then(|o| o.as_str())
.unwrap_or("");
let name = owner
.as_object()
.and_then(|o| o.get("name"))
.and_then(|o| o.as_str())
.unwrap_or("");

if login.is_empty() {
continue;
}

result.push(CrateOwner {
avatar: avatar.to_string(),
email: email.to_string(),
login: login.to_string(),
name: name.to_string(),
});
}
}
.and_then(|j| j.as_array());

let result = if let Some(owners) = owners {
owners
.iter()
.filter_map(|owner| {
fn extract<'a>(owner: &'a Value, field: &str) -> &'a str {
owner
.as_object()
.and_then(|o| o.get(field))
.and_then(|o| o.as_str())
.unwrap_or_default()
}

let avatar = extract(owner, "avatar");
let email = extract(owner, "email");
let login = extract(owner, "login");
let name = extract(owner, "name");

if login.is_empty() {
return None;
}

Some(CrateOwner {
avatar: avatar.to_string(),
email: email.to_string(),
login: login.to_string(),
name: name.to_string(),
})
})
.collect()
} else {
Vec::new()
};

Ok(result)
}
2 changes: 1 addition & 1 deletion src/utils/github_updater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ mod test {
assert!(fields.is_ok());

let fields = fields.unwrap();
assert!(fields.description != "".to_string());
assert!(fields.description != "");
assert!(fields.stars >= 0);
assert!(fields.forks >= 0);
assert!(fields.issues >= 0);
Expand Down
2 changes: 1 addition & 1 deletion src/utils/pubsubhubbub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::collections::HashMap;
use reqwest::*;

fn ping_hub(url: &str) -> Result<Response> {
let mut params = HashMap::new();
let mut params = HashMap::with_capacity(2);
params.insert("hub.mode", "publish");
params.insert("hub.url", "https://docs.rs/releases/feed");
let client = Client::new();
Expand Down
43 changes: 23 additions & 20 deletions src/web/builds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,7 @@ pub fn build_list_handler(req: &mut Request) -> IronResult<Response> {
let conn = extension!(req, Pool).get()?;
let limits = ctry!(Limits::for_crate(&conn, name));

let mut build_list: Vec<Build> = Vec::new();
let mut build_details = None;

// FIXME: getting builds.output may cause performance issues when release have tons of builds
for row in &ctry!(conn.query(
let query = ctry!(conn.query(
"SELECT crates.name,
releases.version,
releases.description,
Expand All @@ -92,24 +88,31 @@ pub fn build_list_handler(req: &mut Request) -> IronResult<Response> {
WHERE crates.name = $1 AND releases.version = $2
ORDER BY id DESC",
&[&name, &version]
)) {
let id: i32 = row.get(5);
));

let build = Build {
id,
rustc_version: row.get(6),
cratesfyi_version: row.get(7),
build_status: row.get(8),
build_time: row.get(9),
output: row.get(10),
};
let mut build_details = None;
// FIXME: getting builds.output may cause performance issues when release have tons of builds
let mut build_list = query
.into_iter()
.map(|row| {
let id: i32 = row.get(5);

if id == req_build_id {
build_details = Some(build.clone());
}
let build = Build {
id,
rustc_version: row.get(6),
cratesfyi_version: row.get(7),
build_status: row.get(8),
build_time: row.get(9),
output: row.get(10),
};

build_list.push(build);
}
if id == req_build_id {
build_details = Some(build.clone());
}

build
})
.collect::<Vec<Build>>();

if req.url.path().join("/").ends_with(".json") {
use iron::headers::{
Expand Down
Loading