-
Notifications
You must be signed in to change notification settings - Fork 344
API Proposal: BufferWriter<T> #2177
Comments
Do we need to have all these |
What's the upside to making them extension methods? |
What about |
Same reason we did it for spans and sequence - fewer methods to JIT when type gets used with different Ts |
I see, |
|
I think it might make sense for very low level types like Span. I don't think we should be unnaturally moving members that belong to a type out in general. |
Does Can you please include other types used in the proposal to API listing (like TransformationFormat, IWritable, etc.) |
StandardFormat controls whether its binary or text. |
Why do we need two sets of methods for
|
Yeah, I was thinking about it. We could remove WriteBytes. We need to measure perf impact and see if we have consistent format chars that could mean "BE" and "LE" and not be already taken by existing text formats. |
Should be called: public ref struct BufferedWriter<TBufferWriter> where TBufferWriter : IBufferWriter<byte> Otherwise its a bit weird, why are you passing something that's already a The answer is because you are buffering it; hence the addition of So Also add a static var writer = new BufferedWriter<BufferWriterFormatter<PipeWriter>>(formattter); vs var writer = BufferedWriter.Create(formattter); |
Assigning to @JeremyKuhne |
@benaadams |
One thing I think we should be careful about is that "buffering" and even "buffered" might imply that data is written to some intermediary buffer and then copied to the ultimate destination (this is how buffered streams work). But there is really not any intermediary buffer here and no data copy on Flush. Flush merely advances a "committed pointer". |
|
e.g. wrap your |
The other nit is the field public ReadOnlySpan<byte> NewLine; It basically doubles the size of the struct and adds an additional GC copy barrier? Just for the additional WriteLine convenience methods? public void WriteLine(int value);
public void WriteLine(int value, StandardFormat format=default);
... Would it be better to have it as an private static readonly byte[] s_newLine = Encoding.UTF8.GetBytes(Environment.NewLine);
public void WriteLine(string value) => WriteLine(value, s_newLine);
public void WriteLine(string value, ReadOnlySpan<byte> newLine); |
And finally... Due to it converting to strings to Utf8; should it be Or would there be a |
Perhaps that's the answer? Its an EncodingWriter?
|
Or just Utf8Writer |
Oh, one more thing 🍎... Add Html writers; the current HtmlEncoding in the framework takes a string, jumps though a bunch on non-inlining virtuals, then does some defensive encoding (as it doesn't know what end character encoding it will be in); then returns a string; which you then have to Utf8 encode. If you you are encoding direct to Utf8 then 90% of it can be skipped and you are only really worried about control chars (as they bother infosec people) and So something like public ref struct Utf8Writer<T> where T : IBufferWriter<byte>
{
WriteHtmlEncoded(string text);
WriteHtmlEncoded(ReadOnlySpan<char> text);
WriteHtmlEncoded(Utf8String text);
WriteHtmlEncoded(ReadOnlySpan<Utf8Char> text);
} There's an implementation here if that's of any benefit? |
The prototype of this type is in https://github.com/dotnet/corefxlab/tree/master/src/System.Buffers.ReaderWriter/System/Buffers/Writer
cc: @davidfowl, @pakrym, @GrabYourPitchforks, @ahsonkhan, @joshfree, @terrajobst
The text was updated successfully, but these errors were encountered: