Skip to content
This repository has been archived by the owner on May 1, 2020. It is now read-only.

markdownのパースをwebworkerで行うように #965

Merged
merged 1 commit into from Sep 11, 2019
Merged

markdownのパースをwebworkerで行うように #965

merged 1 commit into from Sep 11, 2019

Conversation

sapphi-red
Copy link
Member

@sapphi-red sapphi-red commented Jul 15, 2019 •
edited

動きはするのですが、
ソースが闇深すぎるというのと
HMRあたりがおかしな挙動をするので
とりあえずドラフトで

#955 からのブランチです

メモ

  • 76bd360: worker一つ、同期するデータの指定が分散
  • d7181ba: 同期するデータの指定をworker側に
  • 69068de: データの読み込みがされずにレンダリングが走ることがあるバグの修正
  • 090e323: Proxy -> EventTarget(EventTargetの存在思いつかなかった)
  • 7ec3ece: hmrが効かないのを修正
  • 25c2fe9: アクティビティがレンダリンクされていないのを修正
  • fb71bec: レンダリングの直列化とAbortControllerを利用したabortの実装(abortしても実行開始済みのレンダリングが止まらない問題)
  • 1d2d1ee: abortとレンダリングをraceしてabortをより早くすることの試み(少しははやくなったたぶん)
  • becb0a7: fb71bec + 1d2d1ee をrevertした上でWorkerの生成ラグがないように常に2つのWorkerを立ち上げてチャンネルが切り替わった地点でそのWorkerを捨てることで瞬間的にabortするように。(アクティビティで一つとさらにもう一つ別のWorkerが存在しているので実際には4つのWorkerが動く)
  • f75b9c9: ピン留めが表示されないのを修正
  • 6845033: ファイル名変更

@sapphi-red
Copy link
Member Author

sapphi-red commented Jul 17, 2019 •
edited

  • HMRがおかしい
  • パースが早いときと遅いときがある(おそらくパース自体ではなく反映のタイミングの問題)
    • 前のパースが終了していない状態で別のチャンネルに移るとそのパースが終了するまでそのチャンネルのパースが走らないため
    • ex)#zdのメッセージのパースがめちゃ遅い (具体的な原因は#zcや#zaを先に読み込んでいたから)
    • 解決案: worker内で直接にしてpromiseをabortできるようにする
  • activityでメッセージが表示されない

@sapphi-red
Copy link
Member Author

できる限り早くabortするようにしました
ただ一つのメッセージがすごい重い場合はたぶんダメです
場合によってはworker作り直すというのも手もありますが、どのときにやるかという判定をどうするかという問題があります…(あとworkerとwindow間の通信を手書きしないといけなくなる)

@sapphi-red
Copy link
Member Author

たぶんこれで問題点ほぼ全部解決できました
ただ、この実装だとスペックが一定以下の端末だとどうなるかわかりません

@sapphi-red
Copy link
Member Author

  • masterブランチ
    master
  • workerブランチ
    worker

環境

Windows 10
Intel i7-8550U
メモリ8GB
SSD

@sapphi-red sapphi-red marked this pull request as ready for review August 3, 2019 06:20
@sapphi-red
Copy link
Member Author

squashしてrebaseして、WIP外しました 🎉

@n-inja
Copy link
Contributor

n-inja commented Aug 3, 2019

#1009 でconflictさせてしまった……
masterの変更を当ててもらえると助かります

@sapphi-red
Copy link
Member Author

しました 👍

@n-inja
Copy link
Contributor

n-inja commented Aug 3, 2019

スマホのほうだと一番上まで遡ってしまうレベルでちょっとまずい気がします

@sapphi-red
Copy link
Member Author

これで直ったりしますか？

@sapphi-red sapphi-red force-pushed the impr/markdown-worker branch 2 times, most recently from 3cddac8 to 5225579 Compare August 4, 2019 12:37
@sapphi-red sapphi-red force-pushed the impr/markdown-worker branch 2 times, most recently from 58cc095 to e4a3179 Compare August 26, 2019 14:54
@sapphi-red
Copy link
Member Author

スクロールの改善のところまでrebaseしてそれに合わせました
最終メッセージが長い場合少しだけ上にずれるのですがそれ以外は問題なさそうです
safari/iosはevent targetがpolyfillなので少々不安ですが…

n-inja
n-inja approved these changes Sep 11, 2019
View reviewed changes
Copy link
Contributor

@n-inja n-inja left a comment

Choose a reason for hiding this comment

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

👍

@n-inja n-inja merged commit b938199 into master Sep 11, 2019
@n-inja n-inja deleted the impr/markdown-worker branch September 11, 2019 23:35
n-inja added a commit that referenced this pull request Sep 13, 2019
@n-inja
Revert "Merge pull request #965 from traPtitech/impr/markdown-worker"
af6a57d 
This reverts commit b938199, reversing
changes made to ff8ad10.
@sapphi-red sapphi-red mentioned this pull request Sep 13, 2019
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@n-inja n-inja n-inja approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@sapphi-red @n-inja