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

Kornél Pál kornelpal at gmail.com
Mon Jan 11 18:58:03 EST 2010


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


More information about the Mono-devel-list mailing list