Skip to content

Commit

Permalink
refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
Byron committed Aug 14, 2020
1 parent fd2e5ba commit b4a6e16
Show file tree
Hide file tree
Showing 15 changed files with 49 additions and 43 deletions.
9 changes: 7 additions & 2 deletions DEVELOPMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@
* ...even if that includes only the most common usecases.
* **Prefer to increment major version rapidly...**
* ...instead of keeping major version zero for longer than needed.
* **stability**
* we adhere to semantic versioning
* while below 1.0, expect a greater amount of breaking changes, which are announced with minor versions
* From 1.0, we will try hardest to keep the API and user interface non-breaking the closer to the user a library is. Thus the CLI should remain at version
1 for a long times. However, crates that make it up can change more rapidly and may see more major version changes over time.

### Guidelines

Expand All @@ -30,15 +35,15 @@
* `blocking` can be used to make `Read` and `Iterator` async, or move any operation onto a thread which blends it into the
async world.
* Most operations are fast and 'interrupting' them is as easy as ignoring their result by cancelling their task.
* Long-running operations can be roughly interacted with using `git_features::interruptible::interrupt()` function, and after a moment
* Long-running operations can be roughly interacted with using `git_features::interrupt::trigger()` function, and after a moment
of waiting the flag can be unset with the `…::uninterrupt()` function to allow new long-running operations to work.
Every long running operation supports this.
* **server-side**
* Building a pack is CPU and at some point, IO bound, and it makes no sense to use async to handle more connections - git
needs a lot of resources and threads will do just fine.

* **interruption of long-running operations**
* Use `git-features::interruptible::*` for building support for interruptions of long-running operations only.
* Use `git-features::interrupt::*` for building support for interruptions of long-running operations only.
* It's up to the author to decide how to best integrate it, generally we use a poll-based mechanism to check whether
an interrupt flag is set.
* **this is a must if…**
Expand Down
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ Please see _'Development Status'_ for a listing of all crates and their capabili
* [ ] multi-ack
* [ ] multi-ack detailed
* [ ] [server-response (pack)](https://github.com/git/git/blob/master/Documentation/technical/pack-protocol.txt#L404:L404)
* [ ] [side-band mode](https://github.com/git/git/blob/master/Documentation/technical/pack-protocol.txt#L467:L467)
* [ ] push
* [ ] [Version 2](https://github.com/git/git/blob/master/Documentation/technical/protocol-v2.txt)

Expand Down Expand Up @@ -264,7 +265,7 @@ Once installed, there are two binaries:
* **use async IO everywhere**
* for the most part, git operations are heavily relying on memory mapped IO as well as CPU to decompress data,
which doesn't lend itself well to async IO out of the box.
* Use `blocking` as well as `git-features::interruptible` to bring operations into the async world and to control
* Use `blocking` as well as `git-features::interrupt` to bring operations into the async world and to control
long running operations.
* When connecting or streaming over TCP connections, especially when receiving on the server, async seems like a must
though, but behind a feature flag.
Expand Down Expand Up @@ -340,7 +341,7 @@ All feature toggles are additive.
CPUs that support it, like AMD Ryzen or Intel Core i3.
* **interrupt-handler**
* Listen to interrupts and termination requests and provide long-running operations tooling to allow aborting the input stream.
* **Note that** `git_features::interruptible::init_interrupt_handler()` must be called at the start of the application.
* **Note that** `git_features::interrupt::init_handler()` must be called at the start of the application.
* If unset, these utilities will be a no-op which may lead to leaking temporary files when interrupted.
* If the application already sets a handler, this handler will have no effect.

Expand Down
30 changes: 15 additions & 15 deletions git-features/src/interruptible.rs → git-features/src/interrupt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ mod _impl {
sync::atomic::{AtomicUsize, Ordering},
};

pub fn init_interrupt_handler(mut message_channel: impl io::Write + Send + 'static) {
pub fn init_handler(mut message_channel: impl io::Write + Send + 'static) {
ctrlc::set_handler(move || {
const MESSAGES: &[&str] = &[
"interrupt requested",
Expand All @@ -19,7 +19,7 @@ mod _impl {
"if the program doesn't respond quickly enough, please let us know here: https://github.com/Byron/gitoxide/issues"
];
static CURRENT_MESSAGE: AtomicUsize = AtomicUsize::new(0);
if !super::is_interrupted() {
if !super::is_triggered() {
CURRENT_MESSAGE.store(0, Ordering::Relaxed);
}
let msg_idx =CURRENT_MESSAGE.fetch_add(1, Ordering::Relaxed);
Expand All @@ -33,9 +33,9 @@ mod _impl {
mod _impl {
use std::io;

pub fn init_interrupt_handler(_message_channel: impl io::Write + Send + 'static) {}
pub fn init_handler(_message_channel: impl io::Write + Send + 'static) {}
}
pub use _impl::init_interrupt_handler;
pub use _impl::init_handler;

pub struct Read<R> {
pub inner: R,
Expand All @@ -46,7 +46,7 @@ where
R: io::Read,
{
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
if is_interrupted() {
if is_triggered() {
return Err(io::Error::new(io::ErrorKind::Other, "interrupted by user"));
}
self.inner.read(buf)
Expand All @@ -55,13 +55,13 @@ where

static IS_INTERRUPTED: AtomicBool = AtomicBool::new(false);

pub fn is_interrupted() -> bool {
pub fn is_triggered() -> bool {
IS_INTERRUPTED.load(Ordering::Relaxed)
}
pub fn interrupt() {
pub fn trigger() {
IS_INTERRUPTED.store(true, Ordering::Relaxed);
}
pub fn uninterrupt() {
pub fn reset() {
IS_INTERRUPTED.store(false, Ordering::Relaxed);
}

Expand All @@ -70,24 +70,24 @@ pub fn uninterrupt() {
///
/// Note that this is inherently racy and that this will only work deterministically if there is only one
/// top-level function running in a process.
pub struct ResetInterruptOnDrop {
pub struct ResetOnDrop {
was_interrupted: bool,
}

impl Default for ResetInterruptOnDrop {
impl Default for ResetOnDrop {
fn default() -> Self {
ResetInterruptOnDrop {
was_interrupted: is_interrupted(),
ResetOnDrop {
was_interrupted: is_triggered(),
}
}
}

impl Drop for ResetInterruptOnDrop {
impl Drop for ResetOnDrop {
fn drop(&mut self) {
if self.was_interrupted {
interrupt()
trigger()
} else {
uninterrupt()
reset()
}
}
}
2 changes: 1 addition & 1 deletion git-features/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![forbid(unsafe_code)]

pub mod hash;
pub mod interruptible;
pub mod interrupt;
pub mod parallel;
pub mod progress;
4 changes: 2 additions & 2 deletions git-odb/src/hash.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use git_features::hash;
use git_features::interruptible::is_interrupted;
use git_features::interrupt::is_triggered;
use git_object::{owned, HashKind};
use std::{io, path::Path};

Expand Down Expand Up @@ -58,7 +58,7 @@ pub(crate) fn bytes_of_file(
bytes_left -= out.len();
progress.inc_by(out.len());
hasher.update(out);
if is_interrupted() {
if is_triggered() {
return Err(io::Error::new(io::ErrorKind::Other, "Interrupted"));
}
}
Expand Down
4 changes: 2 additions & 2 deletions git-odb/src/pack/bundle/write/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use filebuffer::FileBuffer;

use crate::pack;
use git_features::{interruptible, progress, progress::Progress};
use git_features::{interrupt, progress, progress::Progress};
use std::{
io,
path::{Path, PathBuf},
Expand Down Expand Up @@ -56,7 +56,7 @@ impl pack::Bundle {
}));
let data_path: PathBuf = data_file.lock().path().into();
let pack = PassThrough {
reader: interruptible::Read { inner: pack },
reader: interrupt::Read { inner: pack },
writer: Some(data_file.clone()),
};
let eight_pages = 4096 * 8;
Expand Down
6 changes: 3 additions & 3 deletions git-odb/src/pack/index/traverse/indexed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{
pack::index::{self, util::index_entries_sorted_by_offset_ascending},
pack::tree::traverse::Context,
};
use git_features::interruptible::{interrupt, ResetInterruptOnDrop};
use git_features::interrupt::{trigger, ResetOnDrop};
use git_features::{parallel, progress::Progress};
use git_object::owned;

Expand All @@ -28,15 +28,15 @@ impl index::File {
) -> Result<(), E>,
E: std::error::Error + Send + Sync + 'static,
{
let _reset_interrupt = ResetInterruptOnDrop::default();
let _reset_interrupt = ResetOnDrop::default();
let (verify_result, traversal_result) = parallel::join(
{
let pack_progress = root.add_child("SHA1 of pack");
let index_progress = root.add_child("SHA1 of index");
move || {
let res = self.possibly_verify(pack, check, pack_progress, index_progress);
if res.is_err() {
interrupt();
trigger();
}
res
}
Expand Down
6 changes: 3 additions & 3 deletions git-odb/src/pack/index/traverse/lookup.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::{Error, Reducer, SafetyCheck};
use crate::pack::{self, data::decode, index, index::util};
use git_features::interruptible::ResetInterruptOnDrop;
use git_features::interrupt::ResetOnDrop;
use git_features::{
parallel::{self, in_parallel_if},
progress::{self, unit, Progress},
Expand Down Expand Up @@ -31,15 +31,15 @@ impl index::File {
&mut <<P as Progress>::SubProgress as Progress>::SubProgress,
) -> Result<(), E>,
{
let _reset_interrupt = ResetInterruptOnDrop::default();
let _reset_interrupt = ResetOnDrop::default();
let (verify_result, traversal_result) = parallel::join(
{
let pack_progress = root.add_child("SHA1 of pack");
let index_progress = root.add_child("SHA1 of index");
move || {
let res = self.possibly_verify(pack, check, pack_progress, index_progress);
if res.is_err() {
git_features::interruptible::interrupt();
git_features::interrupt::trigger();
}
res
}
Expand Down
4 changes: 2 additions & 2 deletions git-odb/src/pack/index/traverse/reduce.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::pack::{data::decode, index::traverse};
use git_features::{interruptible::is_interrupted, parallel, progress::Progress};
use git_features::{interrupt::is_triggered, parallel, progress::Progress};
use std::time::Instant;

fn add_decode_result(lhs: &mut decode::Outcome, rhs: decode::Outcome) {
Expand Down Expand Up @@ -87,7 +87,7 @@ where
add_decode_result(&mut self.stats.average, chunk_total);
self.progress.lock().set(self.entries_seen);

if is_interrupted() {
if is_triggered() {
return Err(Self::Error::Interrupted);
}
Ok(())
Expand Down
4 changes: 2 additions & 2 deletions git-odb/src/pack/tree/from_offsets.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{pack, pack::index::access::PackOffset, pack::tree::Tree};
use git_features::{
interruptible::is_interrupted,
interrupt::is_triggered,
progress::{self, Progress},
};
use quick_error::quick_error;
Expand Down Expand Up @@ -105,7 +105,7 @@ impl<T> Tree<T> {
}
};
progress.inc();
if idx % 10_000 == 0 && is_interrupted() {
if idx % 10_000 == 0 && is_triggered() {
return Err(Error::Interrupted);
}
}
Expand Down
4 changes: 2 additions & 2 deletions git-odb/src/pack/tree/traverse/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{
pack::tree::{Item, Tree},
};
use git_features::{
interruptible::is_interrupted,
interrupt::is_triggered,
parallel,
parallel::in_parallel_if,
progress::{self, Progress},
Expand Down Expand Up @@ -127,7 +127,7 @@ where
self.item_count += num_objects;
self.size_progress.inc_by(decompressed_size as usize);
self.progress.lock().set(self.item_count);
if is_interrupted() {
if is_triggered() {
return Err(Error::Interrupted);
}
Ok(())
Expand Down
2 changes: 1 addition & 1 deletion src/plumbing/lean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ fn prepare(
pub fn main() -> Result<()> {
pub use options::*;
let cli: Args = crate::shared::from_env();
git_features::interruptible::init_interrupt_handler(std::io::stderr());
git_features::interrupt::init_handler(std::io::stderr());
let thread_limit = cli.threads;
let verbose = cli.verbose;
match cli.subcommand {
Expand Down
8 changes: 4 additions & 4 deletions src/plumbing/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ fn prepare_and_run<T: Send + 'static>(
) -> Result<T> {
use crate::shared::{self, STANDARD_RANGE};
super::init_env_logger(false);
use git_features::interruptible::{interrupt, is_interrupted};
use git_features::interrupt::{is_triggered, trigger};
match (verbose, progress) {
(false, false) => run(None, &mut stdout(), &mut stderr()),
(true, false) => {
Expand All @@ -192,7 +192,7 @@ fn prepare_and_run<T: Send + 'static>(
let tx = tx.clone();
move || loop {
std::thread::sleep(std::time::Duration::from_millis(500));
if is_interrupted() {
if is_triggered() {
tx.send(Event::UIDone).ok();
break;
}
Expand Down Expand Up @@ -253,7 +253,7 @@ fn prepare_and_run<T: Send + 'static>(
Event::UIDone => {
// We don't know why the UI is done, usually it's the user aborting.
// We need the computation to stop as well so let's wait for that to happen
interrupt();
trigger();
continue;
}
Event::ComputationDone(res, out, err) => {
Expand All @@ -277,7 +277,7 @@ pub fn main() -> Result<()> {
format,
cmd,
} = Args::parse();
git_features::interruptible::init_interrupt_handler(std::io::stderr());
git_features::interrupt::init_handler(std::io::stderr());

match cmd {
Subcommands::IndexFromPack {
Expand Down
2 changes: 1 addition & 1 deletion src/porcelain/lean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use gitoxide_core as core;
pub fn main() -> Result<()> {
pub use options::*;
let cli: Args = crate::shared::from_env();
git_features::interruptible::init_interrupt_handler(std::io::stderr());
git_features::interrupt::init_handler(std::io::stderr());

match cli.subcommand {
SubCommands::Init(_) => core::repository::init(),
Expand Down
2 changes: 1 addition & 1 deletion src/porcelain/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ mod options {
pub fn main() -> Result<()> {
use options::*;
let args = Args::parse();
git_features::interruptible::init_interrupt_handler(std::io::stderr());
git_features::interrupt::init_handler(std::io::stderr());
match args.cmd {
Subcommands::Init => core::repository::init(),
}?;
Expand Down

0 comments on commit b4a6e16

Please sign in to comment.