Skip to content

Commit

Permalink
Merge branch 'async-fetch'
Browse files Browse the repository at this point in the history
  • Loading branch information
Byron committed Nov 14, 2022
2 parents dd94cc5 + 7c4dd21 commit 0c9c48b
Show file tree
Hide file tree
Showing 18 changed files with 301 additions and 73 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

4 changes: 2 additions & 2 deletions etc/check-package-size.sh
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ echo "in root: gitoxide CLI"
(enter git-object && indent cargo diet -n --package-size-limit 25KB)
(enter git-commitgraph && indent cargo diet -n --package-size-limit 30KB)
(enter git-pack && indent cargo diet -n --package-size-limit 120KB)
(enter git-odb && indent cargo diet -n --package-size-limit 120KB)
(enter git-odb && indent cargo diet -n --package-size-limit 130KB)
(enter git-protocol && indent cargo diet -n --package-size-limit 55KB)
(enter git-packetline && indent cargo diet -n --package-size-limit 35KB)
(enter git-repository && indent cargo diet -n --package-size-limit 230KB)
(enter git-transport && indent cargo diet -n --package-size-limit 60KB)
(enter git-transport && indent cargo diet -n --package-size-limit 70KB)
(enter gitoxide-core && indent cargo diet -n --package-size-limit 100KB)
5 changes: 4 additions & 1 deletion git-lock/src/backoff.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::time::Duration;

pub fn randomize(backoff_ms: usize) -> usize {
fn randomize(backoff_ms: usize) -> usize {
let new_value = (fastrand::usize(750..=1250) * backoff_ms) / 1000;
if new_value == 0 {
backoff_ms
Expand All @@ -9,6 +9,7 @@ pub fn randomize(backoff_ms: usize) -> usize {
}
}

/// A utility to calculate steps for exponential backoff similar to how it's done in `git`.
pub struct Exponential<Fn> {
multiplier: usize,
max_multiplier: usize,
Expand All @@ -28,6 +29,7 @@ impl Default for Exponential<fn(usize) -> usize> {
}

impl Exponential<fn(usize) -> usize> {
/// Create a new exponential backoff iterator that backs off in randomized, ever increasing steps.
pub fn default_with_random() -> Self {
Exponential {
multiplier: 1,
Expand All @@ -42,6 +44,7 @@ impl<Transform> Exponential<Transform>
where
Transform: Fn(usize) -> usize,
{
/// Return an iterator that yields `Duration` instances to sleep on until `time` is depleted.
pub fn until_no_remaining(&mut self, time: Duration) -> impl Iterator<Item = Duration> + '_ {
let mut elapsed = Duration::default();
let mut stop_next_iteration = false;
Expand Down
3 changes: 2 additions & 1 deletion git-lock/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ const DOT_LOCK_SUFFIX: &str = ".lock";

///
pub mod acquire;
mod backoff;
///
pub mod backoff;
///
pub mod commit;

Expand Down
3 changes: 2 additions & 1 deletion git-protocol/src/fetch/arguments/async_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ use git_transport::{client, client::TransportV2Ext};
use crate::fetch::{Arguments, Command};

impl Arguments {
pub(crate) async fn send<'a, T: client::Transport + 'a>(
/// Send fetch arguments to the server, and indicate this is the end of negotiations only if `add_done_argument` is present.
pub async fn send<'a, T: client::Transport + 'a>(
&mut self,
transport: &'a mut T,
add_done_argument: bool,
Expand Down
2 changes: 2 additions & 0 deletions git-protocol/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
pub use async_trait;
#[cfg(feature = "futures-io")]
pub use futures_io;
#[cfg(feature = "futures-lite")]
pub use futures_lite;
pub use git_credentials as credentials;
/// A convenience export allowing users of git-protocol to use the transport layer without their own cargo dependency.
pub use git_transport as transport;
Expand Down
5 changes: 3 additions & 2 deletions git-repository/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ blocking-http-transport = ["git-transport/http-client-curl"]
serde1 = [ "serde",
"git-pack/serde1",
"git-object/serde1",
"git-protocol/serde1",
"git-transport/serde1",
"git-protocol?/serde1",
"git-transport?/serde1",
"git-ref/serde1",
"git-odb/serde1",
"git-index/serde1",
Expand Down Expand Up @@ -137,6 +137,7 @@ is_ci = "1.1.1"
anyhow = "1"
walkdir = "2.3.2"
serial_test = "0.9.0"
async-std = { version = "1.12.0", features = ["attributes"] }

[package.metadata.docs.rs]
features = ["document-features", "max-performance", "blocking-network-client", "serde1"]
Expand Down
7 changes: 6 additions & 1 deletion git-repository/src/remote/access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,17 @@ use git_refspec::RefSpec;
use crate::{bstr::BStr, remote, Remote};

/// Access
impl Remote<'_> {
impl<'repo> Remote<'repo> {
/// Return the name of this remote or `None` if it wasn't persisted to disk yet.
pub fn name(&self) -> Option<&str> {
self.name.as_deref()
}

/// Return our repository reference.
pub fn repo(&self) -> &'repo crate::Repository {
self.repo
}

/// Return the set of ref-specs used for `direction`, which may be empty, in order of occurrence in the configuration.
pub fn refspecs(&self, direction: remote::Direction) -> &[RefSpec] {
match direction {
Expand Down
24 changes: 21 additions & 3 deletions git-repository/src/remote/connection/fetch/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,15 @@ where
/// should the fetch not be performed. Furthermore, there the code doing the fetch is inherently blocking so there is no benefit.
/// It's best to unblock it by placing it into its own thread or offload it should usage in an async context be required.
#[allow(clippy::result_large_err)]
pub fn prepare_fetch(mut self, options: ref_map::Options) -> Result<Prepare<'remote, 'repo, T, P>, prepare::Error> {
#[git_protocol::maybe_async::maybe_async]
pub async fn prepare_fetch(
mut self,
options: ref_map::Options,
) -> Result<Prepare<'remote, 'repo, T, P>, prepare::Error> {
if self.remote.refspecs(remote::Direction::Fetch).is_empty() {
return Err(prepare::Error::MissingRefSpecs);
}
let ref_map = self.ref_map_inner(options)?;
let ref_map = self.ref_map_inner(options).await?;
Ok(Prepare {
con: Some(self),
ref_map,
Expand Down Expand Up @@ -165,7 +169,21 @@ where
{
fn drop(&mut self) {
if let Some(mut con) = self.con.take() {
git_protocol::fetch::indicate_end_of_interaction(&mut con.transport).ok();
#[cfg(feature = "async-network-client")]
{
// TODO: this should be an async drop once the feature is available.
// Right now we block the executor by forcing this communication, but that only
// happens if the user didn't actually try to receive a pack, which consumes the
// connection in an async context.
git_protocol::futures_lite::future::block_on(git_protocol::fetch::indicate_end_of_interaction(
&mut con.transport,
))
.ok();
}
#[cfg(not(feature = "async-network-client"))]
{
git_protocol::fetch::indicate_end_of_interaction(&mut con.transport).ok();
}
}
}
}
33 changes: 26 additions & 7 deletions git-repository/src/remote/connection/fetch/receive_pack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,13 @@ where
/// We explicitly don't special case those refs and expect the user to take control. Note that by its nature,
/// force only applies to refs pointing to commits and if they don't, they will be updated either way in our
/// implementation as well.
pub fn receive(mut self, should_interrupt: &AtomicBool) -> Result<Outcome, Error> {
///
/// ### Async Mode Shortcoming
///
/// Currently the entire process of resolving a pack is blocking the executor. This can be fixed using the `blocking` crate, but it
/// didn't seem worth the tradeoff of having more complex code.
#[git_protocol::maybe_async::maybe_async]
pub async fn receive(mut self, should_interrupt: &AtomicBool) -> Result<Outcome, Error> {
let mut con = self.con.take().expect("receive() can only be called once");

let handshake = &self.ref_map.handshake;
Expand Down Expand Up @@ -88,24 +94,28 @@ where
previous_response.as_ref(),
) {
Ok(_) if arguments.is_empty() => {
git_protocol::fetch::indicate_end_of_interaction(&mut con.transport).ok();
git_protocol::fetch::indicate_end_of_interaction(&mut con.transport)
.await
.ok();
return Ok(Outcome {
ref_map: std::mem::take(&mut self.ref_map),
status: Status::NoChange,
});
}
Ok(is_done) => is_done,
Err(err) => {
git_protocol::fetch::indicate_end_of_interaction(&mut con.transport).ok();
git_protocol::fetch::indicate_end_of_interaction(&mut con.transport)
.await
.ok();
return Err(err.into());
}
};
round += 1;
let mut reader = arguments.send(&mut con.transport, is_done)?;
let mut reader = arguments.send(&mut con.transport, is_done).await?;
if sideband_all {
setup_remote_progress(progress, &mut reader);
}
let response = git_protocol::fetch::Response::from_line_reader(protocol_version, &mut reader)?;
let response = git_protocol::fetch::Response::from_line_reader(protocol_version, &mut reader).await?;
if response.has_pack() {
progress.step();
progress.set_name("receiving pack");
Expand All @@ -127,7 +137,14 @@ where

let mut write_pack_bundle = if matches!(self.dry_run, fetch::DryRun::No) {
Some(git_pack::Bundle::write_to_directory(
reader,
#[cfg(feature = "async-network-client")]
{
git_protocol::futures_lite::io::BlockOn::new(reader)
},
#[cfg(not(feature = "async-network-client"))]
{
reader
},
Some(repo.objects.store_ref().path().join("pack")),
con.progress,
should_interrupt,
Expand All @@ -143,7 +160,9 @@ where
};

if matches!(protocol_version, git_protocol::transport::Protocol::V2) {
git_protocol::fetch::indicate_end_of_interaction(&mut con.transport).ok();
git_protocol::fetch::indicate_end_of_interaction(&mut con.transport)
.await
.ok();
}

let update_refs = refs::update(
Expand Down
2 changes: 1 addition & 1 deletion git-repository/src/remote/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,5 @@ mod access;
pub mod ref_map;

///
#[cfg(feature = "blocking-network-client")]
#[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))]
pub mod fetch;
4 changes: 2 additions & 2 deletions git-repository/src/remote/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::bstr::{BStr, BString};

/// If `Yes`, don't really make changes but do as much as possible to get an idea of what would be done.
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
#[cfg(feature = "blocking-network-client")]
#[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))]
pub(crate) enum DryRun {
/// Enable dry-run mode and don't actually change the underlying repository in any way.
Yes,
Expand Down Expand Up @@ -74,5 +74,5 @@ pub struct Mapping {
pub spec_index: usize,
}

#[cfg(feature = "blocking-network-client")]
#[cfg(any(feature = "blocking-network-client", feature = "async-network-client"))]
pub use super::connection::fetch::{negotiate, prepare, refs, Error, Outcome, Prepare, RefLogMessage, Status};

0 comments on commit 0c9c48b

Please sign in to comment.