Skip to content
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

update future-and-promise.md #6

Merged
merged 1 commit into from
Feb 1, 2016

Conversation

saka1
Copy link
Contributor

@saka1 saka1 commented Jan 31, 2016

ちょっとだけ書き直してみました。

  • インクリメントをする際に同期は必ずしも必須ではないように思える
    • Oracle JDKのAtomicIntegerを見ると、読み込んでCAS命令がうまくいくまでぐるぐるループを回す、ロックフリーな実装を使っている様子
  • volatileに関する記述は削ってみた
    • 現代的なScala/Javaでは、あまり使う局面がないような気がするので(本文中に言及があるようにjava,util.concurrent.atomicパッケージのAtomicHogehogeがよりリッチで安全)テキスト的に重要度が低そう

@xuwei-k
Copy link
Contributor

xuwei-k commented Jan 31, 2016

前者の

インクリメントをする際に同期は必ずしも必須ではないように思える

のほうはよさそうなんですが、後者の

volatileに関する記述は削ってみた

に関して、
AtomicHogehogeがよりリッチで安全 もある意味では同意なんですが、
「あまり使う局面がないような気がする 」 程度で削るのは微妙というか、もしなにか明らかな間違いがあるならともかく、似たようなことになった場合にきりがないので、(現状の記述がすごく適切でよいと思っているわけではありませんが)、なにか変えるにしても、もう少し説得力のある理由が欲しい気がします。

あとは、単に削る以外にも

  • 逆に(場合によっては別途コラム的な形で?)詳しくより正確な記述を足す?
  • 書ききれないか、レベル的にはここに書くにはそれほど適切ではないとしたら、volatileに関して詳しく書かれている、参照するべき本かURLへのリンクを貼っておく

などの方法もあると思うので、"単に削る" というのでいいのか、といった論点も

@hexx
Copy link
Collaborator

hexx commented Feb 1, 2016

僕も前半のところは良いと思います
後半は、volatileを使っちゃ駄目だって書いてあるんだから、あってもなくてもどっちでもいいかな
volatileの重要度が低いというのは同意で、これ以上詳しく書く必要はないと思いますw

@lyricallogical
Copy link

変数にvolatileという修飾子をつければいいと考える方もいるかもしれませんが、 volatileは同一のスレッドからしか更新されない場合でしか利用することができず、 今回の用途では利用できないものとなっています

元の説明の「同一のスレッドからしか更新されない場合でしか利用することができず」というのは、分かっている人には、何をいおうとしているのかわかる(人もいると思う)のですが、分かっていない人にとっては「??」だと思うので、削除にしろ修正にしろ、何かしらする必要はあると思います。

書ききれないか、レベル的にはここに書くにはそれほど適切ではないとしたら、volatileに関して詳しく書かれている、参照するべき本かURLへのリンクを貼っておく

書籍としては 「Java Concurrency in Practice」ないしその和訳「Java並行処理プログラミング ―その「基盤」と「最新API」を究める」がよいと思います。「Effective Java」にも確か記述があったはずですが、今ダンボールの中でちょっと確認できない…
Web だと http://www.ibm.com/developerworks/jp/java/library/j-jtp06197.html が良くまとまっていると思います。

個人的には「使うな」が良いと思います。使わせない一番の方法は教えないことなので、書かないのが一番良いと思います。scala.annotation.volatile なんて、使ったことある人いますか?Scala のテキストにのせるには、JVM のメモリモデル、スレッドからの変数の可視性や操作の atomic 性について説明するのは正しいのでしょうか。

もっというと、いきなり AtomicInteger を教えるのも問題がある、と思います。まずは、複数のスレッドから参照される変数に対し、同期の必要性を十分に説明した上で、synchronized を使う例から始め、synchronized { shared_variable_int += 1 } について、int に対する "+= 1" と同等の操作を atomic に提供するための API として AtomicInteger もあるので、パフォーマンスが要求される場合には検討しましょう、という流れのほうが良いのではないかと思います。

もっともっというと、AtomicInteger も教えなくていいと思います。それが必要なドメインにいる人間だけが知っていればいいという認識です。

@xuwei-k
Copy link
Contributor

xuwei-k commented Feb 1, 2016

なるほど。
AtomicIntegerまで教えない方向で書き直すとちょっと変更多くて面倒なので、一旦単にvolatile削除でもいい気はしてきました(つまりこのままmerge?)

@xuwei-k
Copy link
Contributor

xuwei-k commented Feb 1, 2016

2016-02-01 10 57 02

内部のこの部分の履歴調べたら、1年前の自分がcommitにたいして同じようなコメントしてたけど、修正わすれてた・・・(という特に役に立たないオマケ情報)

@matsu-chara
Copy link
Contributor

前半は同意します。

後半部分はvolatileに関する部分を削るというのには僕も賛成です。
AtomicIntegerで値を原子的に更新するってなんだろう・・という気持ちになる(読者もいる)と思いますが、そこを(部分的に)書いてもまた別の疑問がどんどん出てくる感じだと思うので、一旦は参考文献リンクを付けて対応するのがテキストの分量的にも適切そうだと思いました。

参考文献リンクを(脚注などで)付け足す作業はこちらで行って、頂いたプルリクはこのままマージ、というのが良いかなと思いますがいかがでしょう?

@kmizu
Copy link
Contributor

kmizu commented Feb 1, 2016

volatileまず使わないし、削除という方向には賛成です。というわけで、マージしますね。

kmizu pushed a commit that referenced this pull request Feb 1, 2016
@kmizu kmizu merged commit 4bce1ef into scala-text:master Feb 1, 2016
@saka1 saka1 deleted the small-update-future-and-promise branch November 20, 2018 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants