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

AiScript UI APIがメモリリークを引き起こす #15173

Open
1 task done
takejohn opened this issue Dec 24, 2024 · 9 comments
Open
1 task done

AiScript UI APIがメモリリークを引き起こす #15173

takejohn opened this issue Dec 24, 2024 · 9 comments
Assignees
Labels
👩‍💻AiScript Issues related to "AiScript" and its API. "Play" or "Plugin" related issues are not included. 🐛Bug Unexpected behavior

Comments

@takejohn
Copy link

takejohn commented Dec 24, 2024

💡 Summary

Playやスクラッチパッド、ウィジェットのAiScriptにおいて、Ui:C名前空間下の関数を呼び出すと、そのUIコンポーネントが不要となってもオブジェクトがメモリに残り続けます。
このことにより、Ui:C以下の関数を呼び出してコンポーネントを新規作成し、UIを更新すると、メモリリークが発生する可能性があります。

この問題を発生させるAiScriptコード:

for let i, 100000 {
  Ui:C:mfm({
    text: i.to_str()
  })
}
Mk:dialog("実行完了", "")

実行前後のメモリの使用量:
Image

Ui:C名前空間下の関数によって実行される関数createComponentInstanceRef<AiUiComponent>の配列にコンポーネントを追加しますが、
Playスクラッチパッドウィジェットで定義されたその配列から要素が削除されることは無いようです。

🥰 Expected Behavior

Ui:C名前空間下の関数を呼び出しても不要となった後にメモリが解放される

🤬 Actual Behavior

Ui:C名前空間下の関数を呼び出すと、リロードするまでメモリが確保されたままになる

📝 Steps to Reproduce

  1. 開発者ツールなどを用いてメモリ使用量の記録を開始
  2. Playで以下のスクリプトを実行
for let i, 5000 {
  Ui:C:text({
    text: i.to_str()
  })
}
  1. 記録を終了
  2. メモリ使用量を見るとメモリが解放できていないことがわかる

💻 Frontend Environment

* Model and OS of the device(s): OMEN by HP 25L Gaming Desktop GT15-2xxx, Microsoft Windows 11 Home 10.0.26100
* Browser: Google Chrome 131.0.6778.205
* Server URL: misskey.io
* Misskey: 2024.5.0-io.4d

🛰 Backend Environment (for server admin)

* Installation Method or Hosting Service:
* Misskey:
* Node:
* PostgreSQL:
* Redis:
* OS and Architecture:

Do you want to address this bug yourself?

  • Yes, I will patch the bug myself and send a pull request
@takejohn takejohn added the ⚠️bug? This might be a bug label Dec 24, 2024
@samunohito samunohito added 👩‍💻AiScript Issues related to "AiScript" and its API. "Play" or "Plugin" related issues are not included. 🐛Bug Unexpected behavior and removed ⚠️bug? This might be a bug labels Dec 24, 2024
@takejohn
Copy link
Author

やります
(難しかったらやめる)

@takejohn
Copy link
Author

takejohn commented Dec 25, 2024

型エラーが多くてつらい
先に別PRでAiScript関連コードの型エラーに対処してから作業したい

@FineArchs
Copy link
Contributor

Ui:getを使ってUiコンポーネントをidから取得できるという仕様があるので、使われていないものを削除すると齟齬が生じる場合があります

@takejohn
Copy link
Author

takejohn commented Dec 25, 2024

Ui:getを使ってUiコンポーネントをidから取得できるという仕様があるので、使われていないものを削除すると齟齬が生じる場合があります

確かに
IDが指定されたコンポーネントについては残す必要がありそう

@FineArchs
Copy link
Contributor

確かに
IDが指定されたコンポーネントについては残す必要がありそう

IDが指定されていないコンポーネントもUi:C:関数の返り値からIDを取得できるようになっているので、そちらも残す必要があります

@takejohn
Copy link
Author

IDが指定されていないコンポーネントもUi:C:関数の返り値からIDを取得できるようになっているので、そちらも残す必要があります

ならこの問題は解決できないのか…… 🤯

@kakkokari-gtyih
Copy link
Contributor

kakkokari-gtyih commented Dec 25, 2024

dispose関数的なものを用意するとか(指定したIDのコンポーネントを削除する)

悪意あるプログラムがメモリを食いつぶす問題は避けられないけど、通常のプログラムのパフォーマンス改善には使えるかもしれない

@takejohn
Copy link
Author

別issue立てたほうがいいかもしれないけど

let c1 = Ui:C:text({ text: '1' }, 'id')
let c2 = Ui:C:text({ text: '2' }, 'id')
Ui:render([c1, c2])

配列内のidが一致した最初のコンポーネントが使用されるから、こういうコードを実行すると1つ目のコンポーネントだけが表示されるけど、どういう挙動するのが理想なんだろう

@FineArchs
Copy link
Contributor

私は2回目のUi:C:textで更新されてどっちも2になるイメージでした
idが被ったらエラーにするのもアリではありますが、コンポーネントを種類ごと変えたい時もあると思うので

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👩‍💻AiScript Issues related to "AiScript" and its API. "Play" or "Plugin" related issues are not included. 🐛Bug Unexpected behavior
Projects
Development

No branches or pull requests

4 participants