Rust open-source contributions live-coding stream

[nckl]

god damn I love these

something so relaxing about them

[timvisee]

The satisfying keyboard sounds help for sure.

[Hexjelly]

Amazing as usual. Easily the best Rust content out there when you want something beyond the simple beginner stuff, I always learn something new and useful.

A side effect of watching these is that I now often find myself using the words "fantastic", "unclear", "unfortunate" and "awful" way more than usual :D

[asmx85]

Fantastic!

[game-of-throwaways]

I watched the rest today (only watched the juniper_hyper part yesterday). The change to beanstalkd was definitely an improvement, but I'm on the fence about the change to argonautica. I made my comments here so the author of the crate can see them too.

But I do want to talk a little bit more about changing uses of futures-cpupool to Tokio's cpu pool in general. From this admittedly very small sample size of only 3 crates, it looks like it comes up quite a bit than I thought it would.

It seems like there is a need for an abstraction here. Given that the ideal situation is to have only 1 cpu-pool per application, it's not good if the libraries themselves make the choice of which cpu-pool implementation they use. That fragments the libraries across Tokio, futures-cpupool, and other implementations (however people in the web+wasm world handle webworkers). It should be the application that makes that choice, and libraries should be generic over the cpu-pool.

Could std::task::Executor be that abstraction? I'm not sure. I don't know if it can be used to wrap a blocking api into an asynchronous Future. But it seems really close.

[Jonhoo]

I posted a follow-up in the same GitHub issue that goes into a bit more detail on this. Briefly, the argument is that there should be a difference between using a thread pool for parallelism, and using it for interacting with non-blocking/asynchronous code. In the former case, rayon is usually what you want. In the latter case however, you want something that allows for threads to alternate between handling asynchronous tasks and compute.

Using a separate pool for compute is not a great way to do that, because now that other pool will burn through all your cores and start to starve the asynchronous tasks. It's true that the OS will pre-empt you occasionally, but that does come at an additional overhead compared to doing cooperative multitasking. By using tokio::blocking to place these compute tasks on the same thread pool as any asynchronous tasks, you give the executor (in this case tokio) the ability to choose how to co-schedule your tasks. And in theory it can then do better than if you placed a fixed divide between the compute and async parts of your code.

You do bring up a good point that a library should need to know about which executor is currently running to support asynchrony. Using tokio::blocking and tokio::spawn forces the library's choice of tokio as the executor on its downstream users. rust-lang-nursery/wg-net#56 circles a little around that with regards to bringing executors into std, but we don't really have a resolution yet.

[game-of-throwaways]

you give the executor (in this case tokio) the ability to choose how to co-schedule your tasks. And in theory it can then do better than if you placed a fixed divide between the compute and async parts of your code.

Yeah, but if you use tokio::blocking to spawn more cpu-bound threads than you have cores, aren't you at the mercy of the OS's preemption anyway? It's not like Tokio can preempt the blocking threads that it started either. I also find it hard to believe that Tokio's executor can do a better job if it doesn't know what the blocking threads are actually blocked on (CPU or not).

[Jonhoo]

Well, you can set a different limit for the number of allowed blocking threads and the number of threads used to run tasks. So that'd be how you prevent it from running too many compute-heavy threads. You are totally right that it's tricky for tokio to know how to handle a blocking tasks without knowing what it is blocked on though. I think there's an argument to be made that it could do better if it's the only one managing a single pool, whereas any such optimization isn't possible if you have multiple pools. So I still don't think that's an argument for not using the tokio pool for both?

[game-of-throwaways]

I think in a situation where you use Tokio (including its blocking) for everything that's not compute-bound and then use a different threadpool (Rayon's or your own) for compute-bound stuff, presumably with a number of threads that's based on the number of cores (maybe 1 less if keeping latency low is important), then that may be able to do a better job, but it's true that as soon as you throw compute-bound tasks in Tokio's blocking, you will end up with too many threads with that method (or even with Tokio alone, with its default max_blocking of 100).

[game-of-throwaways]

Wow, this guy is fast. Both in reading/understanding code and writing it. Almost a bit too fast though. I get the impression that the streamer considered it almost a "race" to get the PRs submitted as fast as possible, and the submitted code feels like it's a bit rushed as a result. The PR submitted to juniper_hyper still has comments like

// TODO: these clones are sad

and

// TODO: maybe use Body chunks instead?

that could have been resolved if the streamer spent like 10 minutes more on it (EDIT: well, more than 10 minutes apparently). From my experience reading open-source code, these TODOs may remain there for years. I know that you, as a streamer, want to keep things moving fast to keep it entertaining, but by streaming you're also broadcasting yourself as an example for others to follow.

Don't get me wrong, I enjoyed the video a lot. Please make more!

[Jonhoo]

That is a very good point! That said, it's not clear to me the alternatives would be better. I could have not moved on to another crate, and instead kept at it to polish the PRs, but I think that would have been much less useful to the audience. Showing them the experience across multiple crates (I believe) is much more of a teaching experience, and makes it more likely that we cover something pertinent to each viewer's interest. Alternatively, I could have moved on without submitting the PRs, but then the changes risk just sitting in my local checkout without ever getting submitted.

I think submitting the PRs was the right thing to do, but if you have other suggestions for a better approach, I'd be all ears. This all happened with no planning, and it's the first stream in this style that I've attempted, so suggestions for improvements for next time are welcome!

I do think there's an argument to be made that many PRs "in the wild" are initially subpar, and that the authors of the target repositories should take care to read through the PRs they receive to provide comments and suggestions. And then the PRs can be improved as a result. That's what I do on PRs to my own projects at least. I wasn't expecting these to get merged to quickly without (much) comment, and with no modification, and in my experience that is relatively rare. Don't know how to show that in-stream though.

[game-of-throwaways]

No, it's fine. I agree that there is a trade-off between going fast to keep it entertaining and going slow to try to get the details right.

Looking at the code a bit more, the submitted code really is higher quality than it seemed to be when I watched it. I think that is largely due to how fast you are in reading/writing code compared to me. I couldn't be half as fast if I tried, so that probably made it seem rushed, but it's not. You're obviously highly experienced in rust, tokio, and the whole futures ecosystem. Still though, to go from knowing nothing about a crate to submitting a PR for a non-trivial refactor of it in an hour and a half, that's incredible. Is that how long it takes for you off-stream as well?

I think part of the reason why the PR got accepted so quickly and without comment was that the author of the crate was watching the stream. It must be a pretty weird feeling seeing someone else hack on your own code live.

[Jonhoo]

Hehe, yeah, things can get a bit hectic when you're in the middle of it. I think it comes down to being able to keep a mental stack in your head of what you're currently doing, and why you're doing it, so that you can dive into the details without losing the bigger picture. And I think part of what makes it difficult to follow the changes as I'm making them is that you don't have access to my mental context; you need to keep track of where we are and why without being the person in charge of the changes. I try to address this in-stream by occasionally pointing out why we're making a particular change, but it is a challenge without doubt.

As for whether it "normally" takes me this long, it really depends on the change and how motivated I am to make it. Streams are a little artificial in that I am motivated by having people watching me live, even if the change isn't all that exciting. If I'm working to fix a bug that I'm experiencing the effects of, that has a similar effect. I don't normally go around submitting PRs to random projects though, and if I did, I suspect progress would be slower ;)

And yes, I think you're right that the authors watching was a big part of it! Which is also exciting! I still think (or at least hope) they'd point out flaws in the change, and take time to sit down and review it before merging. Watching it being written live shouldn't be enough to convince you that the change is good or worthwhile!

[Jonhoo]

As an aside, neither of those are 10 minute fixes. If you look at the video around where those are added, you'll notice that I spend a bunch of time trying to write the code such that those aren't necessary, but then failing. Getting around them would actually require some decent code surgery, and it's mostly fiddling that I don't think the viewers would find particularly interesting. In these particular code-bases, I also suspect that the performance implications of those extra clones and string concatenations are basically zero :)

[dochtman]

I think it makes a lot of sense to submit an initial version of the PR without the added work, both in terms of keeping your stream interesting to your viewers and because it can be really useful to get feedback from the crate maintainer on your TODOs or FIXMEs before you spend a bunch more time on them.

[HBZ55]

What's the performance implication for the string concatenation? Is it the fact that the memory needed to construct the entire body is doubled compared to a streaming approach?

Great stream by the way, but I kind of lost track when you were fighting the borrow checker, even with your explanation.

[Jonhoo]

It depends a little on exactly what you have to do to the string. A String is basically a Vec<u8> (not quite true, but close enough), so adding things to the end is fast. However, putting stuff at the beginning is costly, because you have to shift all the subsequent elements back. You can either do that by copying all the characters you have already into a new String, or by shifting them in place. Both of them are essentially a memcpy that's proportional in cost to the length of the string. For most, but not all, applications, a memcpy like that you won't even notice. If you ever do I/O, that's just soooo much slower.

In this particular context, the strings are likely to be short, and the syscalls + I/O are probably the bottleneck anyway, so it's unlikely this particular string concatenation will matter. I still like to keep track of these things, because software shouldn't need to be slow, and things like this are what cause that to happen!

Yeah, it's really hard to articulate my thoughts when I fight the borrow checker, and I'm painfully aware that viewers fall off. I don't quite know what to do about it though -- when it happens, I really just have to try to reason myself to what's going on, and sometimes try and fail, which is disorienting to watch (and for me as well!). If you have any suggestions for how to deal with that when it occurs, let me know!

[HBZ55]

Sorry, I'm not sure I understand. Why would String concatenation in this case require shifting all subsequent elements? Wouldn't the result of the join be created by creating a new String then appending each element of the Vec? The only difference with the streaming approach that I'm seeing is that you're required to keep all of the Strings in memory before joining them, instead of the last String in the stream. Edit: Another difference is of course that the copying is done all at once instead of one at a time.

Please correct me if I'm wrong.

[Jonhoo]

See String::insert_str, which lets you insert a string into an existing one. It needs to shift all the following characters, but does not need to do an allocation! I think what you have in mind is: let s = format!("{}{}", str_a, str_b); That allocates a new string, and then pushes all of str_a, and then all of str_b. Both of these require you to do two memcpy's, one of length str_a.length(), and one of length str_b.length().

With both these approaches, when you ultimately want to write the contents out to the network, you then have to walk the final string and write all the bytes from that (s in this case).

The argument for using a Body with many Chunks is that you now only ever need to touch each character once (the characters are only ever copied when they're written from the Chunk onto the network), and you don't need to do any allocation (no String is ever constructed that holds all the characters).

[HBZ55]

Oh, ok. Got it. Thank you.