Avoid serializing if lockfile does not change (#4945)

## Summary

Avoid serializing and writing the lockfile if a cheap comparison shows
that the contents have not changed.

## Test Plan

Shaves ~10ms off of https://github.com/astral-sh/uv/issues/4860 for me.

```
➜  transformers hyperfine "../../uv/target/profiling/uv lock" "../../uv/target/profiling/baseline lock" --warmup 3
Benchmark 1: ../../uv/target/profiling/uv lock
  Time (mean ± σ):     130.5 ms ±   2.5 ms    [User: 130.3 ms, System: 85.0 ms]
  Range (min … max):   126.8 ms … 136.9 ms    23 runs
 
Benchmark 2: ../../uv/target/profiling/baseline lock
  Time (mean ± σ):     140.5 ms ±   5.0 ms    [User: 142.8 ms, System: 85.5 ms]
  Range (min … max):   133.2 ms … 153.3 ms    21 runs
 
Summary
  ../../uv/target/profiling/uv lock ran
    1.08 ± 0.04 times faster than ../../uv/target/profiling/baseline lock
```
This commit is contained in:
Ibraheem Ahmed 2024-07-09 17:08:27 -04:00 committed by GitHub
parent dc80fdba2c
commit 8c9bd70c71
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 34 additions and 15 deletions

View file

@ -63,11 +63,13 @@ pub async fn read_requirements_txt(
}) })
} }
/// Load the preferred requirements from an existing lockfile, applying the upgrade strategy. /// Load the lockfile from the workspace.
pub async fn read_lockfile(workspace: &Workspace, upgrade: &Upgrade) -> Result<LockedRequirements> { ///
/// Returns `Ok(None)` if the lockfile does not exist, is invalid, or is not required for the given upgrade strategy.
pub async fn read_lockfile(workspace: &Workspace, upgrade: &Upgrade) -> Result<Option<Lock>> {
// As an optimization, skip reading the lockfile is we're upgrading all packages anyway. // As an optimization, skip reading the lockfile is we're upgrading all packages anyway.
if upgrade.is_all() { if upgrade.is_all() {
return Ok(LockedRequirements::default()); return Ok(None);
} }
// If an existing lockfile exists, build up a set of preferences. // If an existing lockfile exists, build up a set of preferences.
@ -77,15 +79,20 @@ pub async fn read_lockfile(workspace: &Workspace, upgrade: &Upgrade) -> Result<L
Ok(lock) => lock, Ok(lock) => lock,
Err(err) => { Err(err) => {
eprint!("Failed to parse lockfile; ignoring locked requirements: {err}"); eprint!("Failed to parse lockfile; ignoring locked requirements: {err}");
return Ok(LockedRequirements::default()); return Ok(None);
} }
}, },
Err(err) if err.kind() == std::io::ErrorKind::NotFound => { Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
return Ok(LockedRequirements::default()); return Ok(None);
} }
Err(err) => return Err(err.into()), Err(err) => return Err(err.into()),
}; };
Ok(Some(lock))
}
/// Load the preferred requirements from an existing lockfile, applying the upgrade strategy.
pub fn read_lock_requirements(lock: &Lock, upgrade: &Upgrade) -> LockedRequirements {
let mut preferences = Vec::new(); let mut preferences = Vec::new();
let mut git = Vec::new(); let mut git = Vec::new();
@ -108,5 +115,5 @@ pub async fn read_lockfile(workspace: &Workspace, upgrade: &Upgrade) -> Result<L
} }
} }
Ok(LockedRequirements { preferences, git }) LockedRequirements { preferences, git }
} }

View file

@ -38,7 +38,7 @@ use crate::{RequiresPython, ResolutionGraph};
/// The current version of the lock file format. /// The current version of the lock file format.
const VERSION: u32 = 1; const VERSION: u32 = 1;
#[derive(Clone, Debug, serde::Deserialize)] #[derive(Clone, Debug, serde::Deserialize, PartialEq, Eq)]
#[serde(try_from = "LockWire")] #[serde(try_from = "LockWire")]
pub struct Lock { pub struct Lock {
version: u32, version: u32,
@ -514,7 +514,7 @@ impl TryFrom<LockWire> for Lock {
} }
} }
#[derive(Clone, Debug)] #[derive(Clone, Debug, PartialEq, Eq)]
pub struct Distribution { pub struct Distribution {
pub(crate) id: DistributionId, pub(crate) id: DistributionId,
sdist: Option<SourceDist>, sdist: Option<SourceDist>,
@ -1321,12 +1321,15 @@ enum SourceWire {
subdirectory: Option<String>, subdirectory: Option<String>,
}, },
Path { Path {
#[serde(deserialize_with = "deserialize_path_with_dot")]
path: PathBuf, path: PathBuf,
}, },
Directory { Directory {
#[serde(deserialize_with = "deserialize_path_with_dot")]
directory: PathBuf, directory: PathBuf,
}, },
Editable { Editable {
#[serde(deserialize_with = "deserialize_path_with_dot")]
editable: PathBuf, editable: PathBuf,
}, },
} }
@ -1430,7 +1433,7 @@ enum GitSourceKind {
} }
/// Inspired by: <https://discuss.python.org/t/lock-files-again-but-this-time-w-sdists/46593> /// Inspired by: <https://discuss.python.org/t/lock-files-again-but-this-time-w-sdists/46593>
#[derive(Clone, Debug, serde::Deserialize)] #[derive(Clone, Debug, serde::Deserialize, PartialEq, Eq)]
struct SourceDistMetadata { struct SourceDistMetadata {
/// A hash of the source distribution. /// A hash of the source distribution.
hash: Hash, hash: Hash,
@ -1444,7 +1447,7 @@ struct SourceDistMetadata {
/// locked against was found. The location does not need to exist in the /// locked against was found. The location does not need to exist in the
/// future, so this should be treated as only a hint to where to look /// future, so this should be treated as only a hint to where to look
/// and/or recording where the source dist file originally came from. /// and/or recording where the source dist file originally came from.
#[derive(Clone, Debug, serde::Deserialize)] #[derive(Clone, Debug, serde::Deserialize, PartialEq, Eq)]
#[serde(untagged)] #[serde(untagged)]
enum SourceDist { enum SourceDist {
Url { Url {
@ -1695,7 +1698,7 @@ fn locked_git_url(git_dist: &GitSourceDist) -> Url {
} }
/// Inspired by: <https://discuss.python.org/t/lock-files-again-but-this-time-w-sdists/46593> /// Inspired by: <https://discuss.python.org/t/lock-files-again-but-this-time-w-sdists/46593>
#[derive(Clone, Debug, serde::Deserialize)] #[derive(Clone, Debug, serde::Deserialize, PartialEq, Eq)]
#[serde(try_from = "WheelWire")] #[serde(try_from = "WheelWire")]
struct Wheel { struct Wheel {
/// A URL or file path (via `file://`) where the wheel that was locked /// A URL or file path (via `file://`) where the wheel that was locked
@ -2047,7 +2050,7 @@ impl From<Dependency> for DependencyWire {
/// ///
/// A hash is encoded as a single TOML string in the format /// A hash is encoded as a single TOML string in the format
/// `{algorithm}:{digest}`. /// `{algorithm}:{digest}`.
#[derive(Clone, Debug)] #[derive(Clone, Debug, PartialEq, Eq)]
struct Hash(HashDigest); struct Hash(HashDigest);
impl From<HashDigest> for Hash { impl From<HashDigest> for Hash {

View file

@ -8,7 +8,7 @@ use uv_dispatch::BuildDispatch;
use uv_distribution::{Workspace, DEV_DEPENDENCIES}; use uv_distribution::{Workspace, DEV_DEPENDENCIES};
use uv_git::ResolvedRepositoryReference; use uv_git::ResolvedRepositoryReference;
use uv_python::{Interpreter, PythonFetch, PythonPreference, PythonRequest}; use uv_python::{Interpreter, PythonFetch, PythonPreference, PythonRequest};
use uv_requirements::upgrade::{read_lockfile, LockedRequirements}; use uv_requirements::upgrade::{read_lock_requirements, read_lockfile, LockedRequirements};
use uv_resolver::{FlatIndex, Lock, OptionsBuilder, PythonRequirement, RequiresPython}; use uv_resolver::{FlatIndex, Lock, OptionsBuilder, PythonRequirement, RequiresPython};
use uv_types::{BuildIsolation, EmptyInstalledPackages, HashStrategy}; use uv_types::{BuildIsolation, EmptyInstalledPackages, HashStrategy};
use uv_warnings::{warn_user, warn_user_once}; use uv_warnings::{warn_user, warn_user_once};
@ -175,7 +175,11 @@ pub(super) async fn do_lock(
}; };
// If an existing lockfile exists, build up a set of preferences. // If an existing lockfile exists, build up a set of preferences.
let LockedRequirements { preferences, git } = read_lockfile(workspace, upgrade).await?; let existing_lock = read_lockfile(workspace, upgrade).await?;
let LockedRequirements { preferences, git } = existing_lock
.as_ref()
.map(|lock| read_lock_requirements(lock, upgrade))
.unwrap_or_default();
// Populate the Git resolver. // Populate the Git resolver.
for ResolvedRepositoryReference { reference, sha } in git { for ResolvedRepositoryReference { reference, sha } in git {
@ -234,8 +238,13 @@ pub(super) async fn do_lock(
// Notify the user of any resolution diagnostics. // Notify the user of any resolution diagnostics.
pip::operations::diagnose_resolution(resolution.diagnostics(), printer)?; pip::operations::diagnose_resolution(resolution.diagnostics(), printer)?;
// Write the lockfile to disk. // Avoid serializing and writing to disk if the lock hasn't changed.
let lock = Lock::from_resolution_graph(&resolution)?; let lock = Lock::from_resolution_graph(&resolution)?;
if existing_lock.is_some_and(|existing_lock| existing_lock == lock) {
return Ok(lock);
}
// Write the lockfile to disk.
let encoded = lock.to_toml()?; let encoded = lock.to_toml()?;
fs_err::tokio::write(workspace.install_path().join("uv.lock"), encoded.as_bytes()).await?; fs_err::tokio::write(workspace.install_path().join("uv.lock"), encoded.as_bytes()).await?;