Skip to content

Commit 6478354

Browse files
authored
publish: Avoid N+1 query in add_dependencies() (#7230)
Before this change we were sending out a query for each added dependency, just to look up the corresponding `id` and check if the dependency exists on crates.io. With longer lists of dependencies this behavior is quite wasteful and commonly referred to as an N+1 query. After this change we send out a single query with all dependency names, save the resulting `name, id` tuples to an in-memory `HashMap` and then use that map inside the loop to check if the corresponding crate exists.
1 parent 3d1e38d commit 6478354

File tree

1 file changed

+12
-4
lines changed

1 file changed

+12
-4
lines changed

src/controllers/krate/publish.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@ use crate::auth::AuthCheck;
44
use crate::background_jobs::{Job, PRIORITY_RENDER_README};
55
use axum::body::Bytes;
66
use crates_io_tarball::{process_tarball, TarballError};
7+
use diesel::connection::DefaultLoadingMode;
78
use diesel::dsl::{exists, select};
89
use hex::ToHex;
910
use hyper::body::Buf;
1011
use sha2::{Digest, Sha256};
12+
use std::collections::HashMap;
1113
use tokio::runtime::Handle;
1214
use url::Url;
1315

@@ -436,6 +438,12 @@ pub fn add_dependencies(
436438
use self::dependencies::dsl::*;
437439
use diesel::insert_into;
438440

441+
let crate_ids = crates::table
442+
.select((crates::name, crates::id))
443+
.filter(crates::name.eq_any(deps.iter().map(|d| &d.name.0)))
444+
.load_iter::<(String, i32), DefaultLoadingMode>(conn)?
445+
.collect::<QueryResult<HashMap<_, _>>>()?;
446+
439447
let new_dependencies = deps
440448
.iter()
441449
.map(|dep| {
@@ -446,9 +454,9 @@ pub fn add_dependencies(
446454
}
447455

448456
// Match only identical names to ensure the index always references the original crate name
449-
let krate:Crate = Crate::by_exact_name(&dep.name)
450-
.first(conn)
451-
.map_err(|_| cargo_err(&format_args!("no known crate named `{}`", &*dep.name)))?;
457+
let Some(&krate_id) = crate_ids.get(&dep.name.0) else {
458+
return Err(cargo_err(&format_args!("no known crate named `{}`", &*dep.name)));
459+
};
452460

453461
if let Ok(version_req) = semver::VersionReq::parse(&dep.version_req.0) {
454462
if version_req == semver::VersionReq::STAR {
@@ -461,7 +469,7 @@ pub fn add_dependencies(
461469

462470
Ok((
463471
version_id.eq(target_version_id),
464-
crate_id.eq(krate.id),
472+
crate_id.eq(krate_id),
465473
req.eq(dep.version_req.to_string()),
466474
dep.kind.map(|k| kind.eq(k)),
467475
optional.eq(dep.optional),

0 commit comments

Comments
 (0)