Runtime: Add Stream.CopyToAsync overload without buffer size

[khellang]

Rationale and Usage

Today, Stream has three CopyToAsync overloads:

public Task CopyToAsync(Stream destination)
public Task CopyToAsync(Stream destination, Int32 bufferSize)
public virtual Task CopyToAsync(Stream destination, Int32 bufferSize, CancellationToken cancellationToken)

If I want to pass a cancellation token, I'm forced to specify a buffer size:

// 81920 is the default buffer size...
source.CopyToAsync(target, 81920, cancellationToken);

One issue with the code above is "What does 81920 mean?". For readability, you could also include argument names:

source.CopyToAsync(target, bufferSize: 81920, cancellationToken: cancellationToken);

This leads to some really verbose code, and in the majority of cases, I just want to stick with the default buffer size. With the proposed overload, the above code would end up looking like this:

source.CopyToAsync(target, cancellationToken);

I realize that this could be implemented as an extension method, but since the default buffer size constant is private, you end up having to copy it and future tweaks would not follow. There's also various optimizations inside CopyToAsync that you'd have to reimplement.

Proposed API

namespace System.IO
{
    public abstract class Stream : MarshalByRefObject, IDisposable
    {
        public Task CopyToAsync(Stream destination)
        public Task CopyToAsync(Stream destination, Int32 bufferSize)
+       public Task CopyToAsync(Stream destination, CancellationToken cancellationToken)
        public virtual Task CopyToAsync(Stream destination, Int32 bufferSize, CancellationToken cancellationToken)
    }
}

Details

• The implementation would just call into the (virtual) overload using the default buffer size.

[danmosemsft]

@khellang good suggestoin, can you reformat into a speclet per the example linked in https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/api-review-process.md ?

[jnm2]

For years I've also wanted to be able to create a new StreamWriter with leaveOpen: true without specifying a buffer size or encoding.

[khellang]

For years I've also wanted to be able to create a new StreamWriter with leaveOpen: true without specifying a buffer size or encoding.

Yes! I think that might be a bit more complicated, as it has a lot more ctors, but I'll see if I can file a proposal for that as well.

[khellang]

@danmosemsft Alright. I've tried to follow the linked speclet as closely as I could. Does it look OK now?

[danmosemsft]

@khellang looks good
@ianhays can you give feedback on the proposal.

[ianhays]

Yeah this would be great. This request feels familiar.. I may have seen it elsewhere but searching doesn't turn anything up, so it may have been a verbal request. Either way, that's another 👍 for this proposal.

[danmosemsft]

OK, next step is to wait for the next review meeting... might be a week or two

[svick]

@jnm2 That's https://github.com/dotnet/corefx/issues/8173.

[khellang]

Did this end up getting reviewed? 👼

[danmosemsft]

@khellang nope, but I'll try to get it brought up tomorrow.
cc @terrajobst

[khellang]

No stress. I just heard some rumors about reviews coming up and thought I'd ask :smile:

[danmosemsft]

@khellang do you want to put up a PR?

[terrajobst]

Looks good as proposed

[terrajobst]

Looks good as proposed

[khellang]

I'll submit a PR. I think I have the code sitting in my fork somewhere :smile:

[khellang]

@stephentoub @danmosemsft Now that dotnet/coreclr#14219 and dotnet/corert#4634 has been merged. What's the next step? Wait for @dotnet-maestro-bot, add tests and expose the API somewhere?

[stephentoub]

What's the next step?

There's a backend process that creates "official builds" from coreclr and corert. Once those are done, dotnet-maestro-bot opens PRs in corefx to move corefx to those latest builds, and once that's merged, the API is available to be consumed in corefx. So for now we wait, and once that's available, the next human step is to expose the API from the System.Runtime.cs ref file and add tests. However, there's currently an infrastructure issue that's been delaying those builds getting produced; it's being worked on, and I expect to see them flowing again soon.

[khellang]

I guess it's time to expose the new API in System.Runtime.cs? 😄