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.

Sign up for GitHub

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

Jump to bottom

nyaizeを投稿時にやる #12030

Closed
syuilo opened this issue Oct 14, 2023 · 35 comments
Closed

nyaizeを投稿時にやる #12030

syuilo opened this issue Oct 14, 2023 · 35 comments
Labels
✨Feature This adds/improves/enhances a feature 🐢Performance Efficiency related issue/PR

Comments

@syuilo
Copy link
Member

syuilo commented Oct 14, 2023

Summary

サーバーサイドでノートをpackするたびにパーサー起動させて変換処理を行うのは若干重いため
@syuilo syuilo added 🐢Performance Efficiency related issue/PR ✨Feature This adds/improves/enhances a feature labels Oct 14, 2023
@syuilo syuilo changed the title nyaizeを投稿時にクライアント側でやる nyaizeを投稿時にやる Oct 14, 2023
@syuilo syuilo closed this as completed in 061e389 Oct 14, 2023
@fruitriin
Copy link
Contributor

isCatはずしたとにに元のテキストが

このコミットの前　> 元に戻る
このコミットのあと > にゃいずされたまま

ってことであってる？

@syuilo
Copy link
Member Author

syuilo commented Oct 14, 2023

そうね

@anatawa12
Copy link
Member

これnyaize先にされるせいで検索引っかからなくなったりしませんか？

@syuilo
Copy link
Member Author

syuilo commented Oct 14, 2023

検索用はnyaize前のものを入れれば問題なさそう

@syuilo
Copy link
Member Author

syuilo commented Oct 14, 2023

ただ検索のことなどを心配するのであればそもそもnyaize使わない方が良さそう

@anatawa12
Copy link
Member

どっちかというとnyaizeする人の投稿を検索(含アンテナ)できなく(しづらく)なる方向で気になる人間です(私はnyaizeしする人じゃないけど)

個人的にはdbのデータ量増えちゃうけどnyaizedTextとかDBにはやして検索ではtextとかにできたりしないかなとか思ってます

@sim1222
Copy link
Contributor

sim1222 commented Oct 14, 2023

個人的には検索やアンテナからは除外して欲しくないです。
ユーザーはnyaizeされることを了承した上でCatにしているはずなので、nyaizeによって検索結果に出にくくなる（文字が一致しない）以外の意図的な制約は不要だと考えています。

@syuilo
Copy link
Member Author

syuilo commented Oct 14, 2023

クライアントで表示時に変換すれば解決しそう(すでにクライアントではparseしているためパフォーマンスコストは発生しない)

@syuilo
Copy link
Member Author

syuilo commented Oct 14, 2023

クライアントが対応してないとnyaizeされないデメリットはある

@fruitriin
Copy link
Contributor

nyze前のテキストとnyze後のテキストを両方DBにいれとくのが個人的にはうれしみ

@syuilo
Copy link
Member Author

syuilo commented Oct 14, 2023

それはあまりに非効率な気がする

@fruitriin
Copy link
Contributor

nyzeされたものだけをDBに残すととオリジナルのテキストに戻らないことだけが気がかり

@syuilo
Copy link
Member Author

syuilo commented Oct 14, 2023 •
edited
Loading

nyzeされたものだけをDBに残すととオリジナルのテキストに戻らないことだけが気がかり

クライアントで表示時に変換すれば解決しそう(すでにクライアントではparseしているためパフォーマンスコストは発生しない)

で解決しそう

@anatawa12
Copy link
Member

私は元の問題意識である検索/アンテナ時にNyaize前で引っかかるようなのでクライアントでいいと思ってます

@mei23
Copy link
Contributor

mei23 commented Oct 14, 2023

クライアント依存で変換するようにしたら人々を困らせることが出来なくなるわ

@syuilo
Copy link
Member Author

syuilo commented Oct 14, 2023

それもデメリットとしてあるんだけどパフォーマンスを犠牲にすることなくnyaizeする方法がそれくらいしかなさそう

@syuilo
Copy link
Member Author

syuilo commented Oct 14, 2023

昔のような単純な文字列置換ならパースしなくていいからパフォーマンスへの影響は無視できてたんだけど

@syuilo
Copy link
Member Author

syuilo commented Oct 14, 2023

あと猫耳付けたいけどnyaizeしたくないというクレームがやっぱり多くて人間を困らせない方が良いかもしれなくなってきた
クライアントでやればnyaizeするかどうかを自由に決められるからそれにも対応できる(ただ猫耳云々に関しては構想中のアバターデコレーション機能で解決できるけど)

@CAT5NEKO
Copy link

nyaizeを識別するために猫耳つけて遊んでるんだったら、分離したらただの飾りになっちゃう気がする。
後々アイコンのカスタムアイテムとして拡充していくんだったらありだと思うけど...

@acid-chicken
Copy link
Member

acid-chicken commented Oct 14, 2023 •
edited
Loading

https://github.com/misskey-dev/mfm.rs 活用したら速くなる説

@fruitriin
Copy link
Contributor

今後のMFMの拡張にmfm.jsとmfm.rsのメンテが必要になってめんどくさいのでは（あとmfmのパースをサーバーサイドで行いたくないのでは）

@acid-chicken
Copy link
Member

fn 構文がある以上将来的に変更を加える可能性は少ないのと、どうせ Swift 実装とかそのうち作らないといけないので統一はどの道無理

@mei23
Copy link
Contributor

mei23 commented Oct 14, 2023

今回3→2になったけど

  1. 投稿前クライアントで行う (クライアントで1回負荷, 困る人間=全員, オプトアウトできる人間=投稿者)
  2. 投稿時サーバーで行う (サーバーで1回負荷, 困る人間=全員, オプトアウトできる人間=投稿者？)
  3. 送信時サーバーで行う (サーバーで毎回負荷, 困る人間=対応サーバー, オプトアウトできる人間=サーバー実装？)
  4. 表示時クライアントで行う (各クライアントで1回負荷, 困る人間=対応クライアント, オプトアウトできる人間=閲覧者)

負荷の問題はこれでよくて
仮にnyaizeと猫耳を分離したいとか投稿ごとに制御したいという需要がある場合でも1 or 2で実現可能と思ってるのだわ。

結果的に投稿者が全て制御出来るようになって、閲覧者はオプトアウト出来る抜け穴がなくなる。

@mei23
Copy link
Contributor

mei23 commented Oct 14, 2023 •
edited
Loading

2 vs 4だと、現行投稿時にもサーバーでメンション/ハッシュタグ/絵文字のためにMFM解析してるので、MFM解析回数は変わらないと思うのだわ。

このコミットの状態だと、サーバー上でのMFM解析が2回になっちゃってるかもだわ。
061e389
patsedText が解析後オブジェクトだからそんなことなかったわ

@syuilo
Copy link
Member Author

syuilo commented Oct 14, 2023

投稿はそんなに高頻度で行われないから投稿時に何回パースしてもそこまで影響ないんだけど、pack処理はものすごい数実行されるからそこを減らしたい

@syuilo
Copy link
Member Author

syuilo commented Oct 14, 2023

1と2だとDBにnyaize後のものが保存されるから検索に引っかからない問題とかうまく翻訳できなくなる問題がありそう

@syuilo
Copy link
Member Author

syuilo commented Oct 14, 2023

で3は負荷が高い問題があるからそうなると4しかない

@mei23
Copy link
Contributor

mei23 commented Oct 14, 2023 •
edited
Loading

閲覧者はオプトアウト出来る抜け穴がなくなる。

isCatを付けちゃうと、旧バージョン/サードパーティ/他実装はnyaizeしちゃうわね

@syuilo
Copy link
Member Author

syuilo commented Oct 14, 2023

1と2だとDBにnyaize後のものが保存されるから検索に引っかからない問題とかうまく翻訳できなくなる問題がありそう

これに関しては個人的にはそういった不便を含めてのジョーク機能だからそれで良いんじゃないかという気もするけど

@syuilo syuilo reopened this Oct 14, 2023
@mei23
Copy link
Contributor

mei23 commented Oct 14, 2023

検索に引っかからない問題とかうまく翻訳できなくなる問題

思いっきり名詞が変換されたりしない限りはにゃんとなくイケそうな気がしてるわ

@FineArchs
Copy link
Contributor

投稿時にパースだけしてDBには原文とnyaのインデックスを保存ではダメですか？

@syuilo
Copy link
Member Author

syuilo commented Oct 19, 2023

投稿時にパースだけしてDBには原文とnyaのインデックスを保存ではダメですか？

あまりにも無駄な感じがある

@syuilo
Copy link
Member Author

syuilo commented Oct 19, 2023

やっぱりクライアントで表示時にやりたくなってきた

@syuilo syuilo closed this as completed in 30efd93 Oct 19, 2023
@Nanashia
Copy link
Contributor

  1. 送信時サーバーで行う (サーバーで毎回負荷, 困る人間=対応サーバー, オプトアウトできる人間=サーバー実装？)
  2. 表示時クライアントで行う (各クライアントで1回負荷, 困る人間=対応クライアント, オプトアウトできる人間=閲覧者)

単なるアイデアだけど、各TL取得APIで disableNyaizeみたいなパラメタ生やしてWebクライアントの場合はdisableNyaize: true指定でnyaizeスキップ(4)、既存のサードパーティークライアントはデフォルトでnyaizeされたテキストがもらえる(3.)とサードパーティークライアントの実装の負担を減らせないかしら。

現実的に大半の利用者はWebだと思うのでサーバー負荷軽減の目的は達成しつつ、（nyaize対応してない）サードパーティークライアントだとnyaizeされないみたいな利用者視点でバグっぽい挙動をなくせると思った

@syuilo
Copy link
Member Author

syuilo commented Oct 19, 2023

それでも良さそう

kanarikanaru pushed a commit to kanarikanaru/misskey that referenced this issue Nov 9, 2023
@syuilo @kanarikanaru
enhance: nyaizeはクライアントで表示時に行うように
fc2b838 
Resolve misskey-dev#12030
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
✨Feature This adds/improves/enhances a feature 🐢Performance Efficiency related issue/PR
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@Nanashia @syuilo @fruitriin @acid-chicken @anatawa12 @mei23 @sim1222 @CAT5NEKO @FineArchs