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

Support lazy construction for byte fields #435

Open
blackgnezdo opened this issue Apr 20, 2022 · 2 comments
Open

Support lazy construction for byte fields #435

blackgnezdo opened this issue Apr 20, 2022 · 2 comments

Comments

@blackgnezdo
Copy link
Collaborator

Proto-lens currently generates an API taking strict ByteString for bytes fields. This makes for suboptimal performance because the user is required to make a copy which is then internally turned into a Builder again for serializing the message in Data.ProtoLens.Encoding.Bytes. To avoid breaking the APIs we should introduce alternative lenses for setting the bytes fields to lazy values (or even a Builder). A better API design is welcome.

Internally, we should store a Builder. We'll automatically apply Builder.byteString in generated code. This appears to be efficient so I don't expect any performance regression.

There's a possible continuation of this theme for Text but I don't have a pressing need and the story is not as clear.

@awpr @judah @jinwoo WDYT?

@jinwoo
Copy link
Member

jinwoo commented Apr 20, 2022

Sounds like a good change but what do I know about proto-lens internals :)

@TravisWhitaker
Copy link

This makes for suboptimal performance because the user is required to make a copy

I'm not sure if the parsers generated by proto-lens always make copies of ByteStrings, but regardless use of strict ByteString in the generated type definitions doesn't require the underlying bytes to be copied on construction. See for example the implementation of splitAt

Internally, we should store a Builder.

This would be a performance loss for many usage patterns.

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

3 participants