-
Notifications
You must be signed in to change notification settings - Fork 53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Alternate implementation for Rust #8
Conversation
I also took a look at the code and I wonder if using async_trait in the Rust Socket might be causing any performance issue. |
@marc2332 |
I was confused by the number of channels, especially wrt the timeout impl. |
Yeah. I have the same feeling. Current implementation over-uses |
that was the exact thing i was planning to do, but thanks for saving my time! |
i wonder why did he choose this complicated and inefficient route in the first place, and not keep it simple like his go version? maybe stuff like this was the reason why he took 5x more time to write it in rust than go? |
Howdy @pro465 I tried to keep the implementations is close as possible. As I've said many times, it's really hard to write good rust. For you, it seems to be much easier. My assumptions you've been writing rust for at least a little while. I started writing rust in December, and just trying to use it the best I can. Still learning. Other languages do not have such learning curves as this language. As far as this PR goes, on Monday, I'll be glad to take a review. I've already talked to Benny in the discord about this. And I also wrote out while memeing in front of ~500 people ;) |
oh yeah that is fair then. i actually started in ~august when the corona gave me free time. i can imagine the pressure you had to endure while coding in front of 500 people, while also having not used to rust as much. good luck in your rusty journey! |
Yaya! I am stoked to see this code to give the ol +1 to the skill set. |
Thanks. This implementation is still not as optimized as it can be. I've used |
I have intentionally not hyperoptimized the golang version either. Part of this is simple writing of a program and how does it perform. They have almost identical algorithms. Now we have a divergent algorithm, which makes me want to upgrade the golang one to make it significantly better. I'm pretty sure I could 10x the performance on golang |
@devashishdxt Thanks for writing this, you beat me to it. About how long did it take you to write? |
@benwis I think this is a great idea. This can be the start of the optimization version. I am glad to give you merge permissions if you wish to spearhead the rust version. And by spearhead I mean approve or disapprove any of the changes. |
Also, I don't really believe in commenting code, but for this purpose, a learning adventure, I do think commenting would be very good. Any further changes please consider commenting it |
Happy to do it. And I agree comments here would be beneficial |
@ThePrimeagen You really should give axum a try. Never mind the benchmarks, it's worth it just to experience development of a server with a higher level of abstraction in rust to get a real feel for it. Go follows an incredibly conservative mantra to keep the language clean / readable and provides a remarkable stdlib. Rust tends to push as much as possible out to crates / the community for a number of legit reasons. Go is, without a doubt, an amazing language but damn does it get boring. |
I've been writing Rust since last 4 years and now I'm quite comfortable with it. So, it took me around 1-2 hour to implement this. In addition to that, it took me around 2-3 hours to understand the current implementation (which was difficult because of missing comments). In total, it took me 4-5 hours. |
umm how? both of them spawn one task instead of two tasks as your previous rust prog did. i think the prev rust prog was more divergent. |
As replied in the other thread. Gorilla executes one go routine per connection whereas the rust one has one long running tokio::task. Are they the same? No, it was impossible to make exactly the same, but best effort was made to make them near identical. The gorilla server was to be as the server is in rust. It is its own running item in which emits out server connections, i tried to emulate this best effort in rust. You made a PR that removes that, which makes the programs a bit more divergent but they were already divergent due to how things were handled anyways. I am fine making that change, though they are not equivalent. |
ok i guess, but again, the rust version as it is now, does not diverge more, it is actually closer now:
|
@pro465 we are talking past each other. |
message::{Message, MessageType}, | ||
}; | ||
|
||
pin_project_lite::pin_project! { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey I went to https://crates.io/crates/pin-project-lite and it assumes I know what the heck is going on here.
What is this? What does it do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pin-project is useful if you want to create a self referential reference to something in async Rust. Rust requires that any self referential reference have the Pin trait, so that it can't be moved while the task has yielded or is doing other things. pin-project and pin-project-lite provide convenience macros/funcs to Pin things.
https://rust-lang.github.io/async-book/04_pinning/01_chapter.html
https://blog.cloudflare.com/pin-and-unpin-in-rust/
https://docs.rs/pin-project/latest/pin_project/attr.pin_project.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this part of code should really be extracted to/replaced with library, i think tokio_codec may be used here (Via its Framed wrappers), because this shouldn't end in user code :D
Pin projections used here to implement Stream/Sink traits for this struct
As general introduction to Pins, and why they are important, you can look at Amos write-up (He also has other great articles)
https://fasterthanli.me/articles/pin-and-suffering
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blessings.
tons of reading here
pub fn new(addr: SocketAddr, stream: WebSocketStream<TcpStream>) -> Self { | ||
let (sink, stream) = stream.split(); | ||
|
||
Self { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a side note. I greatly dislike implicit returns.
I have to use additional mental overhead to find see if its semi-colonless to understand its returning. I just prefer a nice red return statement so I don't even have to consider whats happening. General justification is that I read LTR but to know if something returns you have to read to the end of the statement then backwards pump that information. It almost turns thing into a little RTL LTR dance. I just find it straining (i remove all things that require an additional overhead).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rust coming from ML family of programming languages, and ML languages are functional
In functional language it looks like this (example is written in jsonnet.org btw):
local add(a, b) = a + b;
local c = 3;
add(1, 2) + c
// and then you can treat "calls" and other things as replacements instead
local c = 3;
1 + 2 + c
// ...
local c = 3;
3 + c
//
3 + 3
//
6
In jsonnet (and almost all other FP languages), syntax of variable declaration is not
local <name> = <value>
but
local <name> = <value>; <result>
Rust inherited many of functional programming things, and those "implicit returns" shouldn't be read like so, it should be read as "result value of current expression", and "explicit returns" as "early return"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and i still don't like implicit returns, but thank you for trying.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But doesn't it go again your thesis in readme?
I want to be able to build the servers in such a way that each one is as "idiomatic"
For Rust code it is idiomatic to make use of implicit returns whenever possible.
Moreover, it's not just about implicit returns, but about the fact that a lot of things can be expressions (unlike other programming languages). For instance, conditional expressions, while
-loops and many others can be the ones.
Not using this ability seems strange in this project: you can write in other code-styles, but it breaks the whole idea of using the best of languages's ecosystem and making it into just another language.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everybody is entitled to their own opinions, but at the end of the day we're doing this for a coding comparison run by him, that he's going to talk about on YT/Twitch. It's not really different from changing the number of tabs/spaces indentation or putting curly brackets on new lines or what have you.
Adding an explicit return vs an implicit one won't change the performance of the language, so I have no problem with respecting this preference.
I don't agree with it, I like being able to return implicitly. I think it makes the code cleaner and more intuitive. But let's focus on the things that actually have an impact and respect his request for the return keyword.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has no bearing on usability and there is great arguments to be made that as little processing as possible your brain needs to do leaves the smallest amount of decision fatigue. I find it fatiguing playing discover my return. Just a point that is pointless to argue about.
i will always bias towards as much pattern matching my brain can do. ;
is a really hard pattern to spot in comparison to return
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, you can't have "explicit" return here anyway, as rustfmt will replace it with "implicit" :D
rust-lang/rustfmt#4359
Also clippy (tool like go vet, not sure about rustc itself) will mark this code with warning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that on the other hand, very purchasable. Don't fight the formatter
self.sink | ||
.close() | ||
.await | ||
.context("failed to close connection") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know much about this context
. Looks like expect, but it must be used for something, wanna give me the high level view of why its nice to use it (in your perspective)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.context is like explicit building of traces
https://docs.rs/anyhow/latest/anyhow/trait.Context.html
You may treat this as
.map_err(|err| format!("{err}\n{context}"))
Or
errors.Wrap(err, context);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've seen Prime setup a custom errortype with this_error, but if you aren't doing that this isn't a bad way to provide context for your errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i have never played with anyhow, but it seems pretty neet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
anyhow
and thiserror
, both are developed by the same author. thiserror
is more useful for library crates and anyhow
is easier to use for applications. anyhow
provides a quick way of adding context to the error and you don't have to create your own error type.
type Error = anyhow::Error; | ||
|
||
fn poll_ready(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> { | ||
let this = self.project(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i assume this is related to pin_project_lite
. It seems interesting, and i assume it must help with async and ownership (afaik pinning is important in relation to async only).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is related to pin-project
. I had to use it because I decided to implement my own async streams here. Of course there are other ways to do this but I thought this was most straight forward.
@chanced so here is my typical learning flow
I am currently on 3 and this is my 4th attempt at playing around with the tungstenite repo. I find it very hard to reason about how things should be done in rust. But i can give this library a shot. Also this PR is very interesting and i am learning a few things. @devashishdxt there are lots of things i just don't have a clear picture on. I really like the connection design but a bit out of the loop on this pin_project_lite business. |
Rust is definitely a language I highly recommend reading books for first. That's a lot coming from me. |
@chanced yeah, those are good. I have read Rust for Rustaceans and some Ohrly book (forget the name) |
https://www.youtube.com/watch?v=DkMwYxfSYNQ (by the author of rust for rustaceans) may help with the pin_project_lite bit. |
watching! I have some builds that are taking 14 minutes right now, so its a great time to get some extra learnings in |
|
||
#[test] | ||
fn test_real_player_creation() { | ||
let player = Player::new(69, -1.0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
This video is one of the best resources to learn pinning. |
A book I can't recommend enough for this type of thing is Zero to Production in Rust There are so many options in the asynchronous space, and until now there haven't been good complete learning resources for it, being the main missing piece from the |
I guess the purpose is served. Closing this PR. |
I wanted to have yours up for a bit and review it next. There technically isn't a reason to close just yet. This is a very nice version and provides a more "Enterprise" way of doing things (I mean this in a good way. Easier to test and separation of concerns) I just only have so much time and was hurt today so I could not do much. Spent only about 2:50 at the computer |
This is great. Let me check it out. The async story is tough in rust, great to hear there are some nice resources out there |
Yeah async is really still fairly fresh in Rust and they're still smoothing off the rough edges. |
@patternseek well i am reading and trying to understand it better, but its pretty great! I bet the recent changes to mutex probably make the code significantly faster as is. Second, B3NNY also produced a very small, similar, version that runs super omega fast. going to do a recap on that soon, and i would like to read this code more. Its quite nice, props to @devashishdxt |
@ThePrimeagen that's awesome looking forward to that, loving all the Rust content on your channel recently |
I saw your video on YouTube and was surprised that the bullet game code written in Rust was not performing as good as (if not better than) Go. This made me curious and I went through the Rust code.
I've created an alternate implementation of the game in Rust which doesn't use any
Mutex
and very fewchannel
s. Hopefully this one should perform better than the current implementation.To compile and run:
cd rust-alternate/ cargo build --release --bin server ./target/release/server start
If you want to change log level:
You can test this using the same
game_player.rs
in current code.