yuk1ty / learning-systems-programming-in-rust

「Rustでもわかるシステムプログラミング」
430 stars 23 forks source link

4-2の実装を行う #35

Closed higumachan closed 3 years ago

higumachan commented 3 years ago

28

の実装を行いました。

goroutineのコードの置き換えの方針は以下で行っています。

go rust
go func() {} tokio::spawn(async {})
chan T (tokio::sync::mpsc::Receiver<T>, tokio::sync::mpsc::Sender<T>)
ch <- value tx.send(value)
value <- ch rx.recv()
context.With* tokio::sync::Notifyとかもろもろを利用して自前で実装

このPRでメインのコントリビューターのAcceptがあった場合に #40 の議論が収束する認識でいます。

higumachan commented 3 years ago

@laysakura @yuk1ty

一旦、WithCancelをGoの実装をある程度模倣して実装してみました。

ほかでもよく利用するならlibに実装したほうがいいかもしれないですね。

laysakura commented 3 years ago

@higumachan 最新のcommitが

Contextは実装していない部分があるのでdead_codeが存在する。

で状況がちょっとわかっていないですが、review readyでしょうか?

Descriptionも古いので更新いただければと思います。

ほかでもよく利用するならlibに実装したほうがいいかもしれないですね。

Goの標準ライブラリにあるようなものを実装した場合には(他で使うか否かにかかわらず) lib に置いて良いと思います。 ただし lib に置くからには最低限 doc test か unit test はほしいところです。

higumachan commented 3 years ago

@laysakura

で状況がちょっとわかっていないですが、review readyでしょうか?

libへの分割を横においておくならば、review readyな状態です。

Goの標準ライブラリにあるようなものを実装した場合には(他で使うか否かにかかわらず) lib に置いて良いと思います。 ただし lib に置くからには最低限 doc test か unit test はほしいところです。

unit testとかはWithValueとかの他のコンテキストも実装してからやったほうが良さそうだと思います。 一旦このPRはこのコードでマージして頂いて別PRでlibへの分割しようかと思うのですがどうでしょうか?

laysakura commented 3 years ago

lib移しは別PRで、了解です。その形でレビューしていきます。

description更新と

image

の更新ボタン押してのレビューリクエストは随時行っていただけるとレビュワー的に助かります。

higumachan commented 3 years ago

そんな機能あったんですね知らなかったです。 了解しました!

higumachan commented 3 years ago

@laysakura

現状の方針ではMutex周りだけは unwrap を利用しています。(他スレッドでpanicになったときは今回はただちにpanicで良いかと考えています。)

higumachan commented 3 years ago

@yuk1ty

Context トレイトにまつわる実装は、ちょっと実装が長くなってきてしまっているのと、別ファイルで細かく実装の意図/場合によっては簡単な利用例をコメントとして欲しいかなと思いました。コメントやdocは日本語でよいですー!

Goの標準ライブラリにあるようなものを実装した場合には(他で使うか否かにかかわらず) lib に置いて良いと思います。 ただし lib に置くからには最低限 doc test か unit test はほしいところです。

unit testとかはWithValueとかの他のコンテキストも実装してからやったほうが良さそうだと思います。 一旦このPRはこのコードでマージして頂いて別PRでlibへの分割しようかと思うのですがどうでしょうか?

こちらで、一旦PRをマージしたあとに別PRで分割しようかと考えていたのですが、このPRでやったほうが良いでしょうか?

yuk1ty commented 3 years ago

こちらで、一旦PRをマージしたあとに別PRで分割しようかと考えていたのですが、このPRでやったほうが良いでしょうか?

了解です、では、別PRで行きましょう!!🙂

laysakura commented 3 years ago

@yuk1ty からreview requestもらいましたが、 @higumachan もう review ready でしょうか? readyになったら遠慮なくrequestボタン押してください 👍

higumachan commented 3 years ago

@laysakura まだ、コメントを付けている途中ですのでもう少々お待ち下さい。

本日中にreadyにします。

higumachan commented 3 years ago

@laysakura

readyになりました。(pending状態だと再度はrequestボタン押せないみたいです。)

また、今回

コンテキストの key/value 機能 コンテキストがツリーを構成する機能・親が子にキャンセルを伝播させる機能 は使ってないように思います。

これらの処理はコードから削除し、libに移す際に(テストやexampleと共に)追加するのもありだと思います。

こちらなんですが、難しい判断だと思いますが、そこまで削除してしまうと現状の実装している理由がわからなくなる(treeにしてる理由とか)気がするので一旦ここらへんはそのままでいこうかと思います。

higumachan commented 3 years ago

@laysakura

レビュー依頼出させてもらいました。

主な変更点は

yuk1ty commented 3 years ago

長い間お疲れさまでした😃 ありがとうございましたー! goroutine の箇所は tokio で問題なさそうですね!

yuk1ty commented 3 years ago

以前やり取りしたとおり、他にも取り組む必要がある内容がある認識ですので、こちらはもうマージしてしまいますね。引き続き、修正等できそうでしたらよろしくお願いします。