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

[FEATURE REQ] Add helper for converting a stream to BinaryData #38141

Open
m-nash opened this issue Aug 10, 2023 · 1 comment
Open

[FEATURE REQ] Add helper for converting a stream to BinaryData #38141

m-nash opened this issue Aug 10, 2023 · 1 comment
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library.
Milestone

Comments

@m-nash
Copy link
Member

m-nash commented Aug 10, 2023

Library name

Azure.Core

Please describe the feature.

BinaryData.FromStream does a copy so its is faster to use the ReadOnlyMemory overload instead.

MemoryStream stream = GetWrittenStream();
BinaryData data = new BinaryData(stream.GetBuffer().AsMemory(0, (int)stream.Position))

The issue with the above code is stream position is a long and memory wants an int. We need to check for this like this.

MemoryStream stream = GetWrittenStream();
BinaryData data = stream.Position > int.MaxValue
                    ? BinaryData.FromStream(stream)
                    : new BinaryData(stream.GetBuffer().AsMemory(0, (int)stream.Position));

BinaryData.FromStream currently will throw if length is too long if we want to throw or we can do the following

MemoryStream stream = GetWrittenStream();
BinaryData data = stream.Position > int.MaxValue
                    ? new BinaryData(stream.ToArray())
                    : new BinaryData(stream.GetBuffer().AsMemory(0, (int)stream.Position));

The reason for the else is we only want to take the perf hit when the length is larger that max int.

The question is should we provide a helper for this to avoid accidently doing a sub optimal thing.

@m-nash m-nash added Azure.Core Client This issue points to a problem in the data-plane of the library. labels Aug 10, 2023
@github-actions github-actions bot added needs-team-triage Workflow: This issue needs the team to triage. Storage Storage Service (Queues, Blobs, Files) labels Aug 10, 2023
@jsquire jsquire removed Storage Storage Service (Queues, Blobs, Files) needs-team-triage Workflow: This issue needs the team to triage. labels Aug 10, 2023
@annelo-msft annelo-msft added this to the Backlog milestone Apr 9, 2024
@annelo-msft
Copy link
Member

Why not make a PR to BinaryData in the dotnet/runtime repo?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library.
Projects
None yet
Development

No branches or pull requests

3 participants