Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can write_event take a &[u8] instead of a Vec<u8>? #412

Open
junglie85 opened this issue Mar 23, 2023 · 4 comments
Open

Can write_event take a &[u8] instead of a Vec<u8>? #412

junglie85 opened this issue Mar 23, 2023 · 4 comments

Comments

@junglie85
Copy link

Problem description
Writing events requires a Vec<u8>.

Suggestions for an improvement
Make writing events requires &[u8] like the byte API.

@vangork
Copy link
Contributor

vangork commented Mar 28, 2023

The writer event call would send the event in a backend tokio task asynchronously other than from the main task. &[u8] can't guarantee the event won't be changed in the main task.

@junglie85
Copy link
Author

How does that differ from the byte api which does take a slice?

pub async fn write(&mut self, buf: &[u8]) -> Result<usize, Error> {

@vangork
Copy link
Contributor

vangork commented Mar 29, 2023

the slice would acutually be cloned into vec right away.

let bytes_to_write = std::cmp::min(buf.len(), EventWriter::MAX_EVENT_SIZE);
let payload = buf[0..bytes_to_write].to_vec();

@junglie85
Copy link
Author

junglie85 commented Mar 29, 2023

It'd be helpful (cognitively) for the interfaces to be the same. I don't mind whether it's a slice or a vec that is passed in, but I think consistency is good.

I'd normally try to pass a slice because it's more flexible. However, where low latency is key, I can also envisage scenarios where a custom allocator might be preferred instead of the global allocator in the owning vec (when the allocator API is stable). In the latter case, passing in a vec might be more expressive of the ownership situation.

Could the API be re-written to support both options to give greatest flexibility?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants