[Bug]: Delay in typing for property pane codeEditor due to main thread being busy

[rishabhrathod01]

Problem

Below demo & metrics were recorded on the application with 265 records in the table - https://release.app.appsmith.com/app/table-widget-slow/slow-filter-63366e4c10f2005612e63d46/edit

Demo showing delay in typing due to main thread being busy

https://user-images.githubusercontent.com/23132741/193206828-8d7d8621-9256-45db-b165-ae2700749f23.mov

Root cause of the delay

Main thread while typing took ~505 ms

The following tasks contributed to the delay

• generateDataTreeWidget - 203 ms
• getPropertiesUpdatedWidget - 196 ms
• CodeEditor updateMarkings - 83 ms

Both generateDataTreeWidget and getPropertiesUpdatedWidget has common getAllPathsFromPropertyConfigWithoutMemo method which took around 193 ms in both cases.

Possible solutions

• [ ] Explore how to improve performance for getAllPathsFromPropertyConfigWithoutMemo
• [ ] Explore moving complete UnEvalTree generation to separate worker
  • [ ] This could help us make the architecture performant for the future to easily support application-level entities + workspace-level entities
• [ ] Explore moving getPropertiesUpdatedWidget to separate worker

While working on this please do consider creating a generic API that could be used to move expensive tasks to a new worker thread and how to maintain the code.

[rishabhrathod01]

@aswathkk can you please check this issue once?
It seems performance has been impacted after this changes https://github.com/appsmithorg/appsmith/pull/16639/files#diff-7fb06bbc14bb619b1fe3f898559d585167462ee35e7705588a4978596e277052

[aswathkk]

Hi @Rishabh-Rathod, 
I tried reverting the change of childHasPanelConfig done in #16639. I did the profiling before reverting and after reverting. 
 
I didn’t notice any drastic change in the time taken to run the getAllPathsFromPropertyConfigWithoutMemo function.

 
 
I tried exploring some ways to optimise the usage of getAllPathsFromPropertyConfigWithoutMemo. But, didn’t succeed.

 I will just write my findings here: 
 
getAllPathsFromPropertyConfigWithoutMemo function iterates through all the properties (including the nested properties) and calculates the paths. For every property or every widget position updates, this function is getting called for all the widgets in the canvas. We are memoizing the function. But, still we are calling it multiple times with a lot of change in parameters.

Number of calls to this function increases as the number of widgets in the canvas increases. And the time taken to execute this function increases as the number of properties increases. 
Note: JSON form can have a lot of properties and nested properties depending on the incoming data.

 
 
In the example app shared by Rishab, the number of records in the table is not the cause for the perf issue, instead the number of columns in the table data which is fed to the JSON form is the issue.

Here's a solution that I propose to improve the performance of getAllPathsFromPropertyConfigWithoutMemo AFAIK, Not all the property updates cause a change in paths. So, these calls will mostly return the same path for each widget unless a property that has the potential to change a path is getting updated.
 Can we store all the paths for every widget globally? May be in the redux store and update these paths only when a path changing event happens? Then these expensive computations inside the function could be avoided.

[bharath31]

@satbir121 @Rishabh-Rathod what is the next step here?

[bharath31]

@Rishabh-Rathod could you please add the next steps here?

[rishabhrathod01]

We will need to experiment with multiple approaches to understand what would be the best solution here

• Update the paths only when events that would modify the paths.
• Rethink how we create the unEvalTree and send it to the worker. ( If a new widget is added instead of sending the unEvalTree of the widget, we could send an event saying widget creation and evaluation would according handle the logic )

We could experiment with other solutions too.