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.

Sign up for GitHub

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

Jump to bottom

Optional support for bytes::Bytes type backing bytes field #341

Merged
merged 16 commits into from
Jul 18, 2020
Merged

Optional support for bytes::Bytes type backing bytes field #341

merged 16 commits into from
Jul 18, 2020

Conversation

danburkert
Copy link
Collaborator

I made a hash of #337; this is that PR with a few extra commits of my own.

@danburkert danburkert mentioned this pull request Jun 14, 2020
Closed
@rolftimmermans
Copy link
Contributor

👍 Looks good!

@danburkert danburkert mentioned this pull request Jul 7, 2020
Closed
@danburkert danburkert merged commit a1cccbc into master Jul 18, 2020
@danburkert danburkert deleted the buf branch July 18, 2020 21:46
@akiradeveloper
Copy link

@danburkert I am looking for an efficient serialization using bytes::Bytes and found this thread. But a slight look at the code finds me that encoding is still copying the buffer entirely but since you use Bytes that is internally just a pair of pointer and len, you should want to avoid copy here as commented.

But I wonder, how do you do this? Do you have some idea to solve this problem?

 buf.put_slice(value.as_slice()); // TODO Avoid copying

@akiradeveloper akiradeveloper mentioned this pull request Oct 11, 2020
Closed
@danburkert danburkert mentioned this pull request Nov 15, 2020
Closed
@ikopylov ikopylov mentioned this pull request Sep 5, 2022
Merged
tvlbot pushed a commit to tvlfyi/tvix that referenced this pull request Jul 22, 2023
@flokli
feat(tvix/store/proto): use Bytes instead of Vec<u8>
4771c02 
Makes use of tokio-rs/prost#341, which makes our
bytes field cheaper to clone.

It's a bit annoying to configure due to
hyperium/tonic#908, but the workaround does
get the job done.

Change-Id: I25714600b041bb5432d3adf5859b151e72b12778
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8975
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@danburkert @rolftimmermans @akiradeveloper