Skip to content
Draft
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
1 change: 1 addition & 0 deletions .spelling
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ HRESULT
https
i.e.
impl
Implementors
impls
infeasible
inlined
Expand Down
2 changes: 1 addition & 1 deletion crates/anyspawn/src/spawner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use crate::handle::JoinHandleInner;
/// println!("Task running!");
/// });
/// handle.await; // Wait for task to complete
///
///
/// # }
/// ```
///
Expand Down
28 changes: 28 additions & 0 deletions crates/bytesbuf_io/src/concurrent_read.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

use std::fmt::Debug;

use bytesbuf::mem::{HasMemory, Memory};

use crate::Read;

/// Allows a [`Read`] source to support multiple concurrent read operations.
///
/// The [`Read`] trait takes `&mut self`, limiting callers to one read at a time.
/// Some I/O endpoints can support many reads in flight simultaneously for higher throughput.
///
/// Implementors return a new independent [`Read`] handle from each call to [`concurrently()`],
/// enabling the caller to drive any number of reads in parallel.
///
/// [`concurrently()`]: ConcurrentRead::concurrently
pub trait ConcurrentRead: HasMemory + Memory + Debug {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the problem is that Read uses mut reference, all we need is an extension for Read trait that allows to get another copy of this instance

it could look like this

pub trait ConcurentRead : Read {
    fn duplicate(&self) -> Self;
}

the benefit is that you don't need Handle type and the trait itself acts like an extension for Read types

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This alternative formulation feels more limiting. Why should we restrict implementations to only returning Self? The current form feels more flexible at no real cost. Or am I missing some hidden cost/complexity?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handle type adds some level of complexity, probably it will look worse in method's signatures

fn open_concurrent_read<P, R>() -> P 
where 
  P: ConcurrentRead<Handle: R>
  R: Read + Send + 'static,
{
//...
}

alternative

fn open_concurrent_read<R>() -> R
where 
  R: ConcurrentRead
{
//...
}

and also the name is ConcurrentRead, but it doesn't really have any read methods, so it's more like ConcurrentReadProvider or something

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't just making the Read implemnet Clone essentially making it concurrent?

This way, you don't even need to introduce new trait anymore.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't just making the Read implemnet Clone essentially making it concurrent?

This also has the limitation that it forces each read to go through a Self. This seems like an unnecessary limitation.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and also the name is ConcurrentRead, but it doesn't really have any read methods, so it's more like ConcurrentReadProvider or something

Agreed but that "provider" style felt too verbose. Do you have shorter alternative name suggestions? Concurrent is a long word :)

Copy link
Copy Markdown
Member

@martintmk martintmk Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also has the limitation that it forces each read to go through a Self. This seems like an unnecessary limitation.

Why is this a problem?

edit: If i read the API properly, you use concurrent method to retrieve the handle. You could do the same thing by cloning the type that implements Read trait and continue using it on other scope.

To me, the flow is almost identical.

Copy link
Copy Markdown
Member Author

@sandersaares sandersaares Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could do the same thing by cloning the type that implements Read trait and continue using it on other scope.

There is no guarantee that the type that implements Read can be cloned. The concept of "clone" is bigger than "concurrent reads" - we should not impose big-picture requirements like "must support cloning" just to enable concurrent reads.

I agree the flow is the same - but this is not a question of what the code to use the type looks like, this is a question of what requirements we impose to support concurrent access. Being able to clone Self feels like a suspiciously large requirement because it affects more than just the "reading" API surface.

That is not to say that I am completely against it but for a general-purpose API it does not feel like a conservative choice that favors stable long-term APIs.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about ReadDemux/ReadDemultiplexer name?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Demux/Demultiplexer implies something completely different in my view (e.g. that the input is multiplexed, which it is not), does not feel appropriate as a name for this abstraction.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is Debug a requirement here? That seems like an anti-pattern to me.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generic code without Debug requirements gets annoying because you cannot debug-print stuff even though 99% of things implement Debug. I feel we can expect all but the most private types to always implement Debug just to make the dev UX easier.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The naming feels strange to be. "Read" is a trait that has read methods on it, whereas this ConcurrentRead doesn't. I was expecting to see "concurrent read methods" (whatever that would be).

This feels more like a clone-like thing. Like ReadClone and WriteClone?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cloning is intentionally not part of the picture - the thing being returned is not a clone, it is a new object that can be used to issue reads concurrently. Open to better names. In the same family of traits we might have "Deferral" capabilities. ReadDefer and ReadConcurrent maybe?

/// The type of [`Read`] handle returned by [`concurrently()`](ConcurrentRead::concurrently).
type Handle: Read + Send + 'static;

/// Returns a new independent [`Read`] handle that can execute one concurrent read.
///
/// Each handle operates independently - multiple handles may have reads in flight at the
/// same time without interfering with each other.
fn concurrently(&self) -> Self::Handle;
}
28 changes: 28 additions & 0 deletions crates/bytesbuf_io/src/concurrent_write.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

use std::fmt::Debug;

use bytesbuf::mem::{HasMemory, Memory};

use crate::Write;

/// Allows a [`Write`] sink to support multiple concurrent write operations.
///
/// The [`Write`] trait takes `&mut self`, limiting callers to one write at a time.
/// Some I/O endpoints can support many writes in flight simultaneously for higher throughput.
///
/// Implementors return a new independent [`Write`] handle from each call to [`concurrently()`],
/// enabling the caller to drive any number of writes in parallel.
///
/// [`concurrently()`]: ConcurrentWrite::concurrently
pub trait ConcurrentWrite: HasMemory + Memory + Debug {
/// The type of [`Write`] handle returned by [`concurrently()`](ConcurrentWrite::concurrently).
type Handle: Write + Send + 'static;

/// Returns a new independent [`Write`] handle that can execute one concurrent write.
///
/// Each handle operates independently - multiple handles may have writes in flight at the
/// same time without interfering with each other.
fn concurrently(&self) -> Self::Handle;
}
4 changes: 4 additions & 0 deletions crates/bytesbuf_io/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
#![doc(html_logo_url = "https://media.githubusercontent.com/media/microsoft/oxidizer/refs/heads/main/crates/bytesbuf_io/logo.png")]
#![doc(html_favicon_url = "https://media.githubusercontent.com/media/microsoft/oxidizer/refs/heads/main/crates/bytesbuf_io/favicon.ico")]

mod concurrent_read;
mod concurrent_write;
mod error;
mod read;
mod read_ext;
Expand All @@ -33,6 +35,8 @@ mod read_futures;
mod write;
mod write_ext;

pub use concurrent_read::ConcurrentRead;
pub use concurrent_write::ConcurrentWrite;
pub use error::{Error, Result};
pub use read::Read;
pub use read_ext::{ReadExt, ReadInspectDecision};
Expand Down
Loading