jonhoo / volley

Volley is a benchmarking tool for measuring the performance of server networking stacks.
MIT License
123 stars 12 forks source link

undefined behavior in the Rust code #8

Closed steveklabnik closed 9 years ago

steveklabnik commented 9 years ago

https://github.com/jonhoo/volley/blob/master/servers/rust/main.rs#L46-L47

This is technically invoking undefined behavior, as you are aliasing &mut pointers, through the transmute.

I'm willing to send in a patch to fix this, but I can't right this moment, and didn't want to forget.

jonhoo commented 9 years ago

The goal of this was to avoid an extra allocation within the loop for either the integer or the buffer. I assume you're thinking of borrowing buf as mutable within a limited sub-scope so that we can operate on challenge as an integer without breaking the aliasing rules?

steveklabnik commented 9 years ago

When I asked @gankro, our resident unsafe expert, he said

19:02 < Gankro> steveklabnik: I would suggest 
        they transmute the u32 to a 
        [u8; 4] directly
19:02 < Gankro> It's a no-op, and any copies 
        should be trivially elided

But, I don't think that would give you your 'integer or buffer' behavior...

jonhoo commented 9 years ago

Hmm, I don't know the Rust internals well enough, but I don't think that would work quite the same way. The performance impact of keeping two pieces of memory around and copying between them is probably tiny, but I'd prefer to avoid it in the inner loop if possible. The prospect of undefined behavior isn't great though, so if this is the only way, then I'll change it.

steveklabnik commented 9 years ago

Basically, the issue is that &mut is a very strong guarantee: there are no other pointers to this particular piece of memory. The optimizer will assume this...

But, thinking about it some more, you should be able to do what @gankro was talking about: before you want to use it as an integer, transmute() to an integer, before you use it as a [u8; 4], transmute() it to that array. transmute() is literally a no-op, so it should be very cheap.

jonhoo commented 9 years ago

Thanks @steveklabnik!

steveklabnik commented 9 years ago

Thank you! It's cool that you can get away with no allocations and that this is safe, even if it does require some trickery.

jonhoo commented 9 years ago

Agreed! The reason I decided to conduct these benchmarks in the first place was because I wanted to figure out what the theoretical upper limits for my memcached clone in Rust should be: https://github.com/jonhoo/rucache

rucache has a bunch of other tricks too, many of which are similar to this buffer trick. In particular, it turns out that it is quite often useful to be able to interpret a sequence of bytes both in a structured and in an unstructured way, particularly when dealing with low-latency network operations. I wish there was a cleaner way of doing it though -- the whole [u8; N] thing feels like a bit of a hack, partially because the N is not necessarily known at compile-time.

steveklabnik commented 9 years ago

Oh neat!

Type-level integers should help in the future with arrays being awkward, it's certainly a thing right now.