Skip to content

Conversation

mneumann
Copy link
Contributor

Please see especially the rewrite of TcpSocketBuf.read which fixes a performance bug.

For every call to the read() function the internal buffer was copied
into a new buffer (minus the bytes copied into the result buffer). When
the internal buffer is large enough, this severely affects performance,
especially when read_line() is used which calls read_byte() (which calls
read()) for each read byte.

For line oriented I/O this wasn't all that bad, because the internal
buffers usually never were very big. The effect is much more visible
once the buffer grows larger.

Now we always first look into the internal buffer and copy as many bytes
as possible (and desired) into the result buffer. If we need more, we
call the socket read function and use the result as the new internal
buffer, then continue to copy from the (new) internal buffer, and so on.
No need to allocate an additional vector. Instead directly push into the
string.
@bstrie
Copy link
Contributor

bstrie commented Jan 25, 2013

For future reference, you can aim pull requests at the incoming branch by clicking on the black box that says mozilla:master and changing it to mozilla:incoming.

@mneumann
Copy link
Contributor Author

ok!

@brson
Copy link
Contributor

brson commented Jan 28, 2013

Thanks! Merged.

@brson brson closed this Jan 28, 2013
@huonw
Copy link
Contributor

huonw commented Jan 28, 2013

Doesn't the read_line change break UTF8 since it changes how the character interpretation happens (each byte is interpreted as an entire code point in the new version, rather than as (possibly) part of a longer sequence of bytes making up a single character): e.g. if someone typed å (code point 0xE5, 0xC3 0xA5 in UTF8)

fn main() {
    // old version
    let b = ~[0xC3u8, 0xA5];
    io::println(str::from_bytes(b)); // prints 'å'

    // new version
    let mut s = ~"";
    str::push_char(&mut s,0xC3 as char);
    str::push_char(&mut s,0xA5 as char);
    io::println(s); // prints 'Ã¥'

}

@brson
Copy link
Contributor

brson commented Jan 29, 2013

@dbaupp Yes, you are right. I will add a test and revert.

@mneumann
Copy link
Contributor Author

I think instead it would be save to do at the end bytes.push(0) followed by a return cast::transmute(bytes).

RalfJung added a commit to RalfJung/rust that referenced this pull request Oct 15, 2025
RalfJung added a commit to RalfJung/rust that referenced this pull request Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants