joplin icon indicating copy to clipboard operation
joplin copied to clipboard

Desktop: Hiding the note preview pane is much slower than showing it for large notes

Open personalizedrefrigerator opened this issue 1 year ago • 7 comments

Operating system

Linux

Joplin version

2.14.12

Desktop version info

Joplin 2.14.12 (dev, linux)

Client ID: 50a95f7d97414363a84473b388d1916d Sync Version: 3 Profile Version: 45 Keychain Supported: No

Revision: 6b5c60b7f (pr/fix-mermaid-save-button-under-graph)

CodeMirror 6 snippets: 0.0.4 Simple Backup: 1.3.5

Current behaviour

  1. Create a new note in markdown mode.
  2. Download the Project Gutenberg Ebook of Frankenstein and paste it ten times into a note.
  3. Click "Toggle editor layout" to cycle through different layouts

Currently, hiding the markdown preview pane is much slower than any of the other layout transitions.

Screen recording:

https://github.com/laurent22/joplin/assets/46334387/94d48308-6d60-41c1-a9bb-54e2970fa915

I can reproduce this both with the legacy CodeMirror 5 editor and the beta CodeMirror 6 editor.

Expected behaviour

Hiding the markdown preview pane should be at least as fast as showing it.

Logs

08:32:52: CommandService::execute: toggleVisiblePanes
11[Violation] 'requestIdleCallback' handler took <N>ms
main-html.js:54 08:32:53: models/Setting: Saving settings...
main-html.js:54 08:32:54: models/Setting: Settings have been saved.
main-html.js:54 08:32:54: CommandService::execute: toggleVisiblePanes
main-html.js:54 08:32:55: models/Setting: Saving settings...
main-html.js:54 08:32:55: models/Setting: Settings have been saved.
main-html.js:54 08:32:55: CommandService::execute: toggleVisiblePanes
main-html.js:54 08:33:02: models/Setting: Saving settings...
main-html.js:54 08:33:02: models/Setting: Settings have been saved.

i would love to work on this problem, since i am new here can you assist me with where can i find code file?

manu-r12 avatar Feb 23 '24 17:02 manu-r12

i would love to work on this problem, since i am new here can you assist me with where can i find code file?

You might find this pull request that fixed a related issue to be helpful. I suspect that the fix for this issue will probably involve similar files.

Screenshot 2024-02-24 at 11 47 24 PM

while i am working on this issue. I wanna know when we click this "toggle editor layout" , what function this triggers.? Any idea or hint would be so helpful

manu-r12 avatar Feb 24 '24 18:02 manu-r12

I wanna know when we click this "toggle editor layout" , what function this triggers.?

Hi!

The "toggle editor layout" button updates noteVisiblePanes in Joplin's app state, this causes props.visiblePanes to change in the markdown editors.

The markdown editor components can be found here:

The issue seems to be present in both.

How the above information might be discovered.

Here's a process that I find helpful when trying to debug issues like this:

  1. Find a UI string related to the action.
    • In this case, hovering over the button shows "toggle editor layout": screenshot: toggle editor layout shown as a tooltip
  2. Search for that UI string (or part of it) in Joplin's codebase.

In this case, I see that it dispatches a NOTE_VISIBLE_PANES_TOGGLE event. Searching for NOTE_VISIBLE_PANES_TOGGLE brings me to the Redux reducer in app.ts.

In the case statement, I see that the noteVisiblePanes property of the app's state is updated:

https://github.com/laurent22/joplin/blob/ae9c8f27da168e74176c309ba31dd990a6be9211/packages/app-desktop/app.reducer.ts#L156

Searching for noteVisiblePanes gives a list of results. One is in NoteEditor.tsx. Because this is an editor-related bug, this seems to be likely related:

https://github.com/laurent22/joplin/blob/ae9c8f27da168e74176c309ba31dd990a6be9211/packages/app-desktop/gui/NoteEditor/NoteEditor.tsx#L460

Searching again for visiblePanes in the NoteEditor folder brings me to lines in v6/CodeMirror.tsx. For example,

https://github.com/laurent22/joplin/blob/ae9c8f27da168e74176c309ba31dd990a6be9211/packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v6/CodeMirror.tsx#L296

Note: v6/CodeMirror.tsx is the beta markdown editor. v5/CodeMirror.tsx is the legacy markdown editor.

Adding console.log statements or breakpoints in Joplin's development tools can then be used to verify that visiblePanes does change when clicking on the toggle button.

Additional suggestions:

  • The Performance tab in Joplin's developer tools can be useful.
    • Note that a long-running Layout task might suggest that changes made to the DOM by Joplin are causing a large amount of work for the browser.
    • In contrast, long-running JavaScript tasks might suggest that the issue is related to rendering.
  • console.timeLog and console.time could be helpful here (for example, to determine which useEffects are taking a long time).

The Performance tab shows a long running task (takes around 10 seconds) which leads to the measureForScrollbars function inside the CodeMirror package (node_modules) itself. It seems like the main problem is that calculating offsetWidth, offsetHeight and other DOM properties is slowing it down for some reason. I wonder what can be done because the long running task is inside the CodeMirror package itself.

cagnusmarlsen avatar Feb 26 '24 08:02 cagnusmarlsen

At least in the CM6 beta editor, the layout performance issue was caused by hiding the note viewer by setting its width to 1px. Using display = "none" instead seems to fix the issue for both editors.

I've opened a similar performance issue related to a similar part of the codebase as this issue: #10008.