[Mono-dev] BufferedStream.ReadByte and WriteByte are extremely inefficient

[Tom Philpot]

Kornel, Alan

Did these patches check out enough to commit to SVN?

Thanks,
Tom

On Jan 11, 2010, at 4:18 PM, Tom Philpot wrote:

> Ah, yes. I forgot to check that m_buffer.Length == 0 is explicitly disallowed in the constructor, so yes, eliminating an extra if will speed things up.
> 
> Here's another patch which is, again, MIT/X11 licensed.
> 
> 
> On Jan 11, 2010, at 3:58 PM, Kornél Pál wrote:
> 
> Hi,
> 
> This actually means 1 == m_buffer.Length that effectively means no
> buffering. As such there is no use to optimize (special case) for 1 ==
> m_buffer.Length.
> 
> The code will continue to function properly in this special case and
> will be faster in more likely (truly buffered) use cases by simply
> eliminating the "if" by using the "else" case for the 1 ==
> m_buffer.Length case as well.
> 
> Kornél
> 
> Alan McGovern wrote:
> Actually, ignore the part about the 1 >= m_buffer.Length. On second
> reading that's fine ;)
> 
> Alan.
> 
> On Mon, Jan 11, 2010 at 11:24 PM, Alan McGovern <alan.mcgovern at gmail.com
> <mailto:alan.mcgovern at gmail.com>> wrote:
> 
>   Hey,
> 
>   This patch does three things:
> 
>   1) Optimises away some allocations - good
>   2) Added extra checks which throw exceptions - without providing
>   testcases - bad
>   3) A fair few whitespace changes - bad
> 
>   Would you be able to submit two patches instead? The first should do
>   the optimisation, the second should add the test+exception for
>   CanRead and CanWrite as well as adding some nunit test cases which
>   show that this is required.
> 
>   Also:
> 
>   + if (1 >= m_buffer.Length) {
>   + return m_stream.ReadByte ();
> 
>   That should really be: if m_buffer.Length == 0 as the case where the
>   Length is >= 0 is already handled. It makes things easier to understand.
> 
>   Thanks,
>   Alan.
> 
>   On Mon, Jan 11, 2010 at 10:53 PM, Tom Philpot <tom.philpot at logos.com
>   <mailto:tom.philpot at logos.com>> wrote:
> 
>       Sorry, for double posting this patch. Snow Leopard's Mail.app w/
>       Exchange support turned my "This is contributed under the
>       MIT/X11 license" into an attachment.
> 
>       For the record, the code in this patch is contributed under the
>       MIT/X11 license
> 
> 
>       _______________________________________________
>       Mono-devel-list mailing list
>       Mono-devel-list at lists.ximian.com
>       <mailto:Mono-devel-list at lists.ximian.com>
>       http://lists.ximian.com/mailman/listinfo/mono-devel-list
> 
> 
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
> 
> <BufferedStream_ReadWriteBytePatch.txt><ATT00001..txt>