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

Release/v2.4.0 beta prepare #1075

Merged
merged 2 commits into from
Oct 12, 2019
Merged

Release/v2.4.0 beta prepare #1075

merged 2 commits into from
Oct 12, 2019

Conversation

KENCHjp
Copy link
Member

@KENCHjp KENCHjp commented Oct 12, 2019

PR の目的

v2.4.0ベータ版リリース用PR

カテゴリ

  • その他

PR の背景

v2.4.0リリースへ向けてのベータ版公開

PR のメリット

PR のデメリット (トレードオフとかあれば)

PR の影響範囲

関連チケット

sakura-editor/management-forum#75

参考資料

https://github.com/sakura-editor/sakura/wiki/%E3%83%AA%E3%83%AA%E3%83%BC%E3%82%B9%E6%99%82%E3%81%AB%E3%82%84%E3%82%8B%E3%81%93%E3%81%A8

@AppVeyorBot
Copy link

Build sakura 1.0.2311 completed (commit a6497ff4f4 by @KENCHjp)

@beru beru added the Release Release作業チケット【ChangeLog除外】 label Oct 12, 2019
Copy link
Contributor

@beru beru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ダウンロード出来るインストーラーのファイル名が以下のようになっています。
sakura-PR1075-build2311-x64-Release-alpha-Installer.zip

ベータ版なので、alpha というのを beta に変える必要があると思います。

@beru
Copy link
Contributor

beru commented Oct 12, 2019

ダウンロード出来るインストーラーのファイル名が以下のようになっています。
sakura-PR1075-build2311-x64-Release-alpha-Installer.zip

ベータ版なので、alpha というのを beta に変える必要があると思います。

zipArtifacts.bat ファイルの中を見ると、x64 の場合は ALPHA という変数経由で RELEASE_PHASE を alpha にしていますね。という事はこの alpha というのは v2.4.0 の alpha, beta とは関係ない別物かもしれません。。

@beru
Copy link
Contributor

beru commented Oct 12, 2019

sakura_core/config/build_config.h を見ると下記の記述が有り

#if _WIN64
#define ALPHA_VERSION
#endif

64bit 版はアルファ版扱いになっていて、バージョン情報画面も下記のように表示されます。

image

@KENCHjp
Copy link
Member Author

KENCHjp commented Oct 12, 2019

たしか、x64はまだ正式リリースじゃないからそうしてるんじゃないかと。

@berryzplus
Copy link
Contributor

たしか、x64はまだ正式リリースじゃないからそうしてるんじゃないかと。

その認識であってます。x64版 はフェーズに関わらずalphaです。

一応「32bitじゃできないことができるのを確認する ⇒ 4GBのファイルを開く」をクリア条件として止めていたはず。確認してないけど「既にできますよね?」という話はできると思ってますが、その話とリリース作業のチェックは別件としたいです。

@beru
Copy link
Contributor

beru commented Oct 12, 2019

たしか、x64はまだ正式リリースじゃないからそうしてるんじゃないかと。

関連する Issue を見てみました。

https://github.com/sakura-editor/sakura/issues?q=is%3Aissue+is%3Aopen+x64+label%3Ax64

#430 を見てみましたが、人によって結構意見が色々ですね。

@beru
Copy link
Contributor

beru commented Oct 12, 2019

たしか、x64はまだ正式リリースじゃないからそうしてるんじゃないかと。

その認識であってます。x64版 はフェーズに関わらずalphaです。

一応「32bitじゃできないことができるのを確認する ⇒ 4GBのファイルを開く」をクリア条件として止めていたはず。確認してないけど「既にできますよね?」という話はできると思ってますが、その話とリリース作業のチェックは別件としたいです。

個人的には「4GBのファイルを開く」をクリア出来ない限り x64 が alpha のまま留まり続けるというのは定義としてどんなもんだろうか?と思いますが…。

Copy link
Contributor

@berryzplus berryzplus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks Good to me...

Copy link
Contributor

@beru beru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

問題無いと思います。

@KENCHjp
Copy link
Member Author

KENCHjp commented Oct 12, 2019

レビューありがとうございます。
では、無事台風が抜けてくれたら続きやります。
こんな時に地震まで来るなんて。。。

@KENCHjp KENCHjp merged commit 8964670 into sakura-editor:release/v2.4.0-beta Oct 12, 2019
@berryzplus
Copy link
Contributor

個人的には「4GBのファイルを開く」をクリア出来ない限り x64 が alpha のまま留まり続けるというのは定義としてどんなもんだろうか?と思いますが…。

#430 では色々書いてますが「開くだけなら既にできている( #430 (comment)) 」ので、解決への道はあると思っています。逆転の発想で、解決に至るための「課題出し」がまったくできないのなら、まだ alpha を取るべきじゃない気がしてます。

@beru
Copy link
Contributor

beru commented Oct 12, 2019

#430 では色々書いてますが「開くだけなら既にできている( #430 (comment)) 」ので、解決への道はあると思っています。逆転の発想で、解決に至るための「課題出し」がまったくできないのなら、まだ alpha を取るべきじゃない気がしてます。

言うは易しなので書くだけ書いてしまいますが(自分が色々と片づけるとは言っていない)内部処理を考慮して要件が厳密に定義されているわけではないし、色々なバリエーションで試験しているわけでも無いので、ちょっと目標が厳密でないというか曖昧な気がします(それじゃあ詳しく要件を列挙してみろと言われるとちょっと面倒ですが…)。

まぁでもユーザー目線だと64bit版なら4GB以上のファイルを開いて編集して保存が出来ないと「対応してないじゃん」って思ってしまいそうなので苦しいですね…。ただ32bit版では2GB以下のファイルを問題無く扱えていたかというとそうではない筈です。「これとそれとは別の話。64bit版では4GB以上のファイルは問題無く扱えないと駄目。」と言い切られてしまうともう言葉を返せません。根拠なくそう主張されると議論が成立しないので。

@beru
Copy link
Contributor

beru commented Oct 12, 2019

まぁ 64bit対応! と謳うからには 32bit 版では実現が難しくて出来てない事も扱えるようにしたい、という考えは分からなくも無いです。ユーザーが理解しにくい制限が残ってると、64bit版なのに何それ?って思うのが自然かもしれないですね…。

@KENCHjp
Copy link
Member Author

KENCHjp commented Oct 12, 2019

盛り上がってますねw

とりあえず、64bit対応はある意味命題なので、何ができたら64bit対応といえるかって項目あげてそれをクリアすれば(それが全て出ないかもしれないけど)Ok.って決めちゃえばいいんじゃないかとおもってます。

とりあえず、4Gファイルを開いて保存する(レスポンスは置いておいて)ができるのが一つのチェックポイントで私はいい気がしてます。

@berryzplus
Copy link
Contributor

今日は他事に忙しくて切れ間切れ間で見てます・・・。

言うは易しなので書くだけ書いてしまいますが(自分が色々と片づけるとは言っていない)

「言うだけの人」の数も圧倒的に足りてないのが現状なので、プロジェクト的には歓迎だと思いますよ 😃

ちょっと目標が厳密でないというか曖昧な気がします

どうしたら厳密な目標を 誰かが考えてくれる状況 になるか?
と考えてすすめてみた試みも過去にあります。 ⇒ sakura-editor/management-forum#71

まぁでもユーザー目線だと64bit版なら4GB以上のファイルを開いて編集して保存が出来ないと「対応してないじゃん」って思ってしまいそうなので苦しいですね…。

たぶんそこは現実的には、どこで諦めるか?の線引きをせめぎあう展開になると思っています。
「軽快に動作しない」は許してくれるんじゃないかなぁ・・・。
最低限、開いたファイルを保存したときに壊すのはマズいっす。

ただ32bit版では2GB以下のファイルを問題無く扱えていたかというとそうではない筈です。

たぶん、32bit版に匹敵するパフォーマンス+アルファがリリースのクリア条件だと思うんですよ。32bit版でできないことはできなくて構わないはずなんだけど、何ができないのか把握できてないっていう・・・。

「これとそれとは別の話。64bit版では4GB以上のファイルは問題無く扱えないと駄目。」と言い切られてしまうともう言葉を返せません。

「別件としたい」と書いたのは、今回のリリース準備に「 x64対応の一時的な完了 」を含められる可能性が「0パーセント」だと思ったからです。一時的にでも「対応完了」としないのに、alphaを外してみるのはなんか違う気がします。現状のままalphaを取れるか?と考えて、もっともらしい理由をつけられる自信はありません。手を動かさなければ alpha は取れないと思っています。

@beru
Copy link
Contributor

beru commented Oct 12, 2019

とりあえず、4Gファイルを開いて保存する(レスポンスは置いておいて)ができるのが一つのチェックポイントで私はいい気がしてます。

4GiB のサイズのファイルを開くのにどれくらいの仮想メモリを必要とするのかをざっとですが試算してみます。

4GiB のファイルの内容は、1行1024文字(改行CRLF込み)で行数は 4,194,304 行とします。使われている文字は全てASCII文字とします。

サクラエディタは内部的に UTF-16LE の文字エンコーディングに変換するので、2倍する必要があります。また行毎に色々な型(CLayout と CDocLine と…他にもあるかな?)のインスタンスが用意されます。sizeof(CDocLine) は 40 で sizeof(CLayout) は 64 なので1行に追加で 104 バイト必要だと考えると、(2048 + 40 + 64) * 4194304 = 9026142208 で、9GiB くらい必要になります。

実際にはもっともっと色々な事にメモリが使われると思いますが(ヒープのチャンクヘッダの分とか)、自分が今思い付く大体の内容で書いてます。正確な値ではありません。

仮に必要なサイズが物理メモリの容量を超えたとしてもページングファイルにスワップ出来るので動作しないという事は無いかもしれませんがパフォーマンスは極端に悪化しそうです。

なおメモリバッファクラス CMemory のデータサイズとバッファサイズのメンバーの型が int なので 1行のデータが 2GiB までしか扱えません。なので 4GiB のどんなファイルも扱えるというわけではありません。

保存にまつわる処理のどこが問題で 4GiB のファイルを正しく保存出来ないのかは把握していません。そもそも読み取りがきちんと行えているかも自分は未確認です。

まぁ色々と検証とか面倒なので、「これぐらいのサイズのファイルを正しく扱えないと 64bit に対応したとは見做せない」という考えは線引きとしてどうかなぁと思います。分かりやすい違いが無いとメリットも感じられないかもしれませんが…。

@beru
Copy link
Contributor

beru commented Oct 12, 2019

どうしたら厳密な目標を 誰かが考えてくれる状況 になるか?
と考えてすすめてみた試みも過去にあります。 ⇒ sakura-editor/management-forum#71

その Issue は記載が長いのでポエムか何かだと捉えてそっ閉じしていました。

たぶんそこは現実的には、どこで諦めるか?の線引きをせめぎあう展開になると思っています。
「軽快に動作しない」は許してくれるんじゃないかなぁ・・・。
最低限、開いたファイルを保存したときに壊すのはマズいっす。

それは確かにそうですよね。ファイルを上書きして壊すぐらいなら保存できないと表明する方が良いと思います。ただどれぐらいのデータなら扱えるかというのを事前にチェックしてから処理している実装ではないのかもしれません(少なくとも徹底はされていないので色々な問題が起きる)。

たぶん、32bit版に匹敵するパフォーマンス+アルファがリリースのクリア条件だと思うんですよ。32bit版でできないことはできなくて構わないはずなんだけど、何ができないのか把握できてないっていう・・・。

パフォーマンスについてはレジスタの本数が x64 で増えているので大体のケースで上回ってるんじゃないかと勝手に思っていますが、計測して比較はしていません。

64bit になってアドレス空間は大分広がったので(ポインタのサイズも増えますが…)32bit 版では扱えなかった大きいデータも本来は実装が適切ならば 64bit 版では扱えるはずなんですよね。ただ現状そうではないと。でもそれをクリアしないと 64bit 対応と見做せないかというとそんな事は無いと思うんです。言葉遊び的な感じですが、対応と活用は違うのではないかと。

「別件としたい」と書いたのは、今回のリリース準備に「 x64対応の一時的な完了 」を含められる可能性が「0パーセント」だと思ったからです。一時的にでも「対応完了」としないのに、alphaを外してみるのはなんか違う気がします。現状のままalphaを取れるか?と考えて、もっともらしい理由をつけられる自信はありません。手を動かさなければ alpha は取れないと思っています。

今回のリリースで x64 を alpha のままにする事に強い抵抗感があるというわけではないです。
ベータ版なのに alpha という表記があると自分みたいに混乱してしまいそうですけど、それはまぁそういうもんだと納得してしまえば良いので。

@takke
Copy link
Member

takke commented Oct 12, 2019

いっそ 64bit 版の表記を alpha ではなく experimental か何かに変えてみるのはどうでしょうかねー。

@berryzplus
Copy link
Contributor

ベータ版のリリースが追加されているのを見ました。
https://github.com/sakura-editor/sakura/releases/tag/v2.4.0-beta

そもそもの話ですが、現状では Win32 - Release のみがリリース対象なので、x64版について語るのは気が早かったのかもです。途中、ぼくもどっちか分かんなくなってましたが、今回はまだ Win32 だけが対象になるのが正しい気がしてます。

(じゃ、x64版をGitHub Releaseするにゃあどうしたらええんや!)

@KENCHjp
Copy link
Member Author

KENCHjp commented Oct 13, 2019

ま、OSSなので、進んではまた直すで、いいんじゃないかと。
ソフトウエアも人間が作るもので唯一無二の正解なんてないのかなと。

@arigayas
Copy link

takke さんが提案した64bit のバージョン表記のAlphaをExperimental にするのはいかがでしょうか?
そうすればベータ版のリリースページにアルファ版がある違和感は解消できると思います。

@beru
Copy link
Contributor

beru commented Oct 13, 2019

v2.4.0.2313 64bit を使って、4GB のファイル(別プログラムで作成)を開いて編集して保存をしてまた開いてみましたが、特に問題なく変更した内容で見れました。

とはいえ操作内容によっては何かの問題が起きるのかもしれないので、問題が無いとは言い切れないです。操作のレスポンスが良くないので色々試すのがなんか大変な気がして試してません。

自分は 64bit 版に固有の致命的な不具合が無いのであれば alpha とかの表記は外して普通にリリースして良いんじゃないかなと思います。リリース前だろうが後だろうが問題が有れば(修正出来るものは)修正する事になるのは変わらないので…。

@KENCHjp
Copy link
Member Author

KENCHjp commented Oct 13, 2019

「Experimental 」いきましょう!

@KENCHjp
Copy link
Member Author

KENCHjp commented Oct 21, 2019

>リリース前だろうが後だろうが問題が有れば(修正出来るものは)修正する事になるのは変わらないので…。

賛成。
で、リリースはやっぱり32bitと同時っすよね。他のOSSを見てても。
今のリリースはいったんこのままで、次は64bitもalphaとりますかね。
たしかなにかissueありましたよね、64bitの。

@m-tmatma m-tmatma added this to the v2.4.0 milestone Dec 29, 2019
@KENCHjp KENCHjp added the no-changelog 【ChangeLog除外】 label Jan 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog 【ChangeLog除外】 Release Release作業チケット【ChangeLog除外】
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants