Conversation
There was a problem hiding this comment.
C'est pas mal du tout ton petit projet !
J'ai fait un pull request bidon à partir du premier commit juste pour avoir l'interface de review.
Pour le choix des crates, tu as pris des libs connues et matures, j'aurai pas fait des choix différents je pense.
Il y a quelques trucs pas idiomatiques, mais franchement mineurs.
Le plus gros changement que je ferai c'est de convertir en async, car on "sent" que le code a été converti à partir de C/C++.
En passant en async, je pense que tu pourrais diviser la taille de ton code par deux, car il serait plus haut niveau. Par contre ça te fait rentrer dans un autre monde, tu serai obligé de réécrire tout ce qui fait des threads (si tu en as encore besoin) ou entrées/sorties en async, de remplacer certaines dépendances, etc.
En même temps, il n'y a pas énormément de code, donc si t'es motivé, ça peut te faire un exercice fun et pas trop chronophage 😉
src/server.rs
Outdated
| loop { | ||
| let (socket, addr) = listener.accept().unwrap(); | ||
| match Client::create(socket) { | ||
| Err(e) => log::warn!("Impossible de connecter le nouveau client {} : {}", addr, e), | ||
| Ok(()) => log::info!("Nouveau client connecte : {}", addr) | ||
| } | ||
| } |
There was a problem hiding this comment.
Mineur : Tu peux simplifier en chaînant les méthodes des itérateurs :
| loop { | |
| let (socket, addr) = listener.accept().unwrap(); | |
| match Client::create(socket) { | |
| Err(e) => log::warn!("Impossible de connecter le nouveau client {} : {}", addr, e), | |
| Ok(()) => log::info!("Nouveau client connecte : {}", addr) | |
| } | |
| } | |
| listener.incoming().map(Result::unwrap).for_each(|socket| { | |
| let addr = socket.peer_addr().unwrap(); | |
| match Client::create(socket) { | |
| Err(e) => log::warn!("Impossible de connecter le nouveau client {} : {}", addr, e), | |
| Ok(()) => log::info!("Nouveau client connecte : {}", addr), | |
| } | |
| }); |
Ou encore plus en déplaçant le logging dans Client::create :
| loop { | |
| let (socket, addr) = listener.accept().unwrap(); | |
| match Client::create(socket) { | |
| Err(e) => log::warn!("Impossible de connecter le nouveau client {} : {}", addr, e), | |
| Ok(()) => log::info!("Nouveau client connecte : {}", addr) | |
| } | |
| } | |
| listener.incoming().map(Result::unwrap).for_each(Client::create); |
src/internal_com.rs
Outdated
| // Serialisation de l'info de trafic dans un buffer pour pouvoir l'envoyer | ||
| let buf = bincode::serialize(traffic_infos).unwrap(); | ||
|
|
||
| // Envoi du buffer sur la socket multicast | ||
| self.socket.send(&buf).unwrap(); |
There was a problem hiding this comment.
Comme Socket implémente le trait Write, tu peux faire directement :
| // Serialisation de l'info de trafic dans un buffer pour pouvoir l'envoyer | |
| let buf = bincode::serialize(traffic_infos).unwrap(); | |
| // Envoi du buffer sur la socket multicast | |
| self.socket.send(&buf).unwrap(); | |
| bincode::serialize_into(self.socket).unwrap(); |
There was a problem hiding this comment.
J'ai l'impression que std::net::UdpSocket n'implemente pas le trait Write alors que std::net::TcpStream l'implémente. Je ne saurai pas te dire pourquoi.
|
|
||
| /// Recoit un datagram | ||
| /// Attention : en cas d'erreur, il faut faire appel a clear pour recevoir un nouveau datagram | ||
| pub fn recv(&mut self, mut sock: &TcpStream) -> anyhow::Result<Option<&[u8]>> { |
There was a problem hiding this comment.
Ce code et Client::work_thread me laisse penser que ton programme bénéficierai fortement à être réécrit en async.
Tu pourrais faire la lecture de la taille, puis la lecture du buffer, dans les deux cas avec read_exact, et retourner que quand tu as un message complet ou une erreur, sans gérer d'état interne intermédiaire, ce qui simplifierai beaucoup la logique. Le runtime ferait le changement de contexte entre les sockets, et l'équivalent du poll, et ton code serait plus haut niveau.
A noter que c'est possible d'être async et mono thread avec tokio, cf. https://docs.rs/tokio/latest/tokio/runtime/index.html#current-thread-scheduler
desbma
changed the title
| // Envoi de l'info de trafic aux clients | ||
| self.sender.send(&traffic_infos); | ||
| } | ||
| match reader.read_event()? { |
There was a problem hiding this comment.
Pour parser du XML avec quick-xml, tu peux utiliser l'API event comme tu as fait (qui déboite pour la perf, car 0-copy), mais tu peux aussi définir une struct pour représenter le document XML, ajouter derive(serde::Deserialize) dessus, implémenter FromStr pour TrafficInfos, et serde fera tout le reste. La deuxième approche est beaucoup plus simple je trouve.
Exemple au hasard : https://github.com/desbma/stfed/blob/master/src/config.rs#L45
…s many pools as CPUs (one thread by pool)
2 participants
No description provided.