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

Desktop: Switching a note using Sidebar is slow and grayed out for a moment #6416

Closed
ken1kob opened this issue Apr 16, 2022 · 4 comments
Closed
Labels
bug It's a bug desktop All desktop platforms high High priority issues

Comments

@ken1kob
Copy link
Contributor

ken1kob commented Apr 16, 2022

As reported in the topic of discourse, the following symptoms are observed:

  • Symptom 1: Switching a note using SideBar is slow and takes longer time than other triggers.
  • Symptom 2: When a note is switched using SideBar, NoteEditor is grayed out for a moment.

The next table is an example of the above symptoms when a note swiched. The measurement conditions are: target note: "1. Welcome to Joplin!", Joplin ver: 2.7.15, CPU: Core i5-4670, and OS: Win10. To use NoteList for a trigger, select "All notes" in SideBar

Condition Plugins Trigger Time (msec) Gray-out
Within a notebook NoteTabs + Outline NoteList 706
Between notebooks NoteTabs + Outline NoteList 727
Between notebooks NoteTabs + Outline Note Tabs plugin 1363
Between notebooks NoteTabs + Outline SideBar 1815
Within a notebook no NoteList 206
Between notebooks no NoteList 212
Between notebooks no SideBar 1150

The example shows interesting observations:

  • Switching by NoteList between notebooks is not so slower than switching within a notebook.
  • On the other hand, switching by SideBar is very slow. It seems there is a large overhead here. Since SideBar is a very common way to switch notes, it should be fast.
  • Switching by Note Tabs plugin is slower than by NoteList. It's natural because the plugin perform several tasks other than note switching. So, it is not reasonable that switching by SideBar is slower than Note Tabs plugin.
  • Only switching by SideBar causes gray-out. Other triggers, NoteList and Note Tabs plugin don't.

What is expected

  • Switching a note using SideBar is as fast as using NoteList.
  • When a note is switched using SideBar, NoteEditor is not grayed out.

Environment

  • Joplin version: any (2.7.x)
  • Platform: any desktop
@ken1kob ken1kob added the bug It's a bug label Apr 16, 2022
@laurent22 laurent22 added desktop All desktop platforms high High priority issues labels Apr 16, 2022
@ken1kob
Copy link
Contributor Author

ken1kob commented Apr 16, 2022

Analysis

To analyze the above observations, I used Performance Analyzer in Electron's DevTool.

The following image is the difference of Performance Analyzer views of note switching (A) using NoteList and (B) using SideBar. Each view are measured without plugins.

pa-compare-sidebar

The findings are:

  • There are more overheads when using SideBar.
  • Since switching using NoteList doesn't change the contents of NoteList but switching using SideBar almost changes NoteList, it is natural that re-rendering tasks of (B) are much more than ones of (A).
  • It is very strange that displaying no note in NoteEditor is processed.
  • Another strange portion is reloading mermaid script. It should not be reloaded in such a frequent situation. It takes near 20% time.

To assess the last item deeper, another analysis was performed. The next image (C) is a view with the same condition with (B) except RichMarkdown and Math Mode plugins are added. Both plugins have large scripts and are loaded at the start of Joplin.

pa-compare-sidebar-c

It shows that both RichMarkdown and Math Mode plugins are also reloaded like mermaid. In conclusion, it is found that for each additional plugin that has a script for Editor/Viewer, it takes more time to switch notes using SideBar. This behavior is not desirable and would be not intended.

@ken1kob
Copy link
Contributor Author

ken1kob commented Apr 17, 2022

Cause

The cause of the symptoms is that no note is selected for a moment after a notebook in SideBar is clicked, even if a non-empty notebook is selected.

Sequence in detail:

  1. A notebook in SideBar is clicked.
  2. No note is selected.
  3. NoteEditor's body component is unmounted from the DOM tree.
  4. The location of NoteEditor is filled in gray (ref. NoteEditor.renderNoNote()).
  5. A note is selected in the selected notebook.
  6. Any re-rendering action for a note are performed.
  7. NoteEditor component is mounted to the DOM tree.
  8. Re-mounting makes all scripts for Markdown Editor and Viewer loaded again.

Why "no note is selected" happens? The reason is clicking Sidebar uses FOLDER_SELECT action for the reducer. The sequence after FOLDER_SELECT action is as follows:

  1. A new notebook gets selected and no note gets selected.
  2. The first note (in the specific sort order) in the new notebook gets selected.

There is some time gap between the step 1 and 2, and some re-renderings are performed in the gap. This transient state of no note causes various side effects, which are sound in logic but redundant and result in the symptoms.

Other triggers such as NoteList use FOLDER_AND_NOTE_SELECT action, and FOLDER_AND_NOTE_SELECT has no gap of no note. So, they are irrelevant to the symptoms.

@ken1kob
Copy link
Contributor Author

ken1kob commented Apr 19, 2022

To Solve the Issue

To solve the issue, several approaches are possible:

  • (A) Re-designing reducer's actions to simultaneously select a notebooks and its first note
  • (B) Using FOLDER_AND_NOTE_SELECT action
  • (C) Ignoring a transient no-note state in GUI

About (A), the impact of a modification will be large and prevail to multiple applications. It is not recommended.

About (B), it is difficult to be implemented, because there is no available information of a target note when a notebook in SideBar is clicked.

About (C), a modification will be small, and its impact will be small and closed only in the desktop app. It seems to be appropriate.

According approach (C), I made PR #6430.

In the above performance analysis, it seems that there are large overheads in re-rendering of SideBar and NoteList. However, even if there are many rooms for improvement, SideBar and NoteList are currently scheduled to be fundamentally refactored. So, I'll hold off on improving them for the time being.

@ken1kob
Copy link
Contributor Author

ken1kob commented Apr 19, 2022

Performance improvement after fixed

Condition Plugins Trigger Before
(msec)
PR#6430
(msec)
PR#6430 + PR#6409
(msec)
Within a notebook no NoteList 206 186 162
Between notebooks no NoteList 212 226 174
Between notebooks no SideBar 1150 752 688
Between notebooks RichMarkdown + Math Mode SideBar 1266 775 687
Within a notebook NoteTabs + Outline NoteList 706 713 219
Between notebooks NoteTabs + Outline SideBar 1815 1632 821

It shows:

  • Note switching using SideBar is greatly improved (shown in bold face).
  • PR#6409 provides further improvement after PR#6430 is applied. They are complementary.

Besides, after PR#6430 is applied, gray-out does not happen in any case.

As mentioned in the previous post, I'm not planning to improve the overheads in re-rendering of SideBar and NoteList now. So, I'll give up that switching using SideBar is as fast as one using NoteList in this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug It's a bug desktop All desktop platforms high High priority issues
Projects
None yet
Development

No branches or pull requests

2 participants