Conversation
| /// Attempt to obtain a read grant. Returns `Ok((start, size))` on success. | ||
| fn read(&self) -> Result<(usize, usize), ReadGrantError>; | ||
|
|
||
| /// Attempt to obtain a split read grant. Return `Ok(((start1, size1), (start2, size2)))` on success. |
There was a problem hiding this comment.
Do you think it's worth adding a default impl to this that always returns an error, in order to avoid a breaking change for now?
There was a problem hiding this comment.
If we DO make a breaking change, I wonder if it's worth making an struct OffsetSlice { offset: usize, len: usize }, and returning Result<[OffsetSlice; 2], ReadGrantError> or something in order to avoid complex tuple types (esp. since both are usizes types!)
There was a problem hiding this comment.
Maybe instead of an error, always return an empty second section?
Though that might be more confusing in terms of debugging in some cases.
There was a problem hiding this comment.
I think that might be "too clever by half", yeah.
There was a problem hiding this comment.
Another idea I just had would be to have a new trait SplitCoord: Coord that has split_read and split_release_inner (which I realized I need too). Then SplitGrantR can be given out only if Q::Coord :SplitCoord.
Adding a default impl would be fine, too. What do you prefer?
I like the OffsetSlice idea.
There was a problem hiding this comment.
I'm open to the specialized trait idea, as long as it looks reasonable in practice! I know we do that with Notify already.
There was a problem hiding this comment.
That being said, I don't think having split grants adds any overhead, like having async notify would. That actually makes me think maybe we don't specialize it? Sorry to give you conflicting opinions, but I'd actually maybe save "specialization" traits for cases where having the specialized version "costs" more.
|
I'm overall open to this, it would be nice to have some basic tests for this functionality (I know the new code isn't great in that regard yet). |
|
Since When you are ready for breaking changes I would change a few things:
|
|
|
||
| /// Mark `used1 + used2` bytes as available for writing. | ||
| /// `used1` corresponds to the first split grant, `used2` to the second. | ||
| fn split_release_inner(&self, _used1: usize, _used2: usize) { |
There was a problem hiding this comment.
I am not super happy with this signature, but I can't put my finger on it...
What do you think?
Fixes #121