perf: Widget re-rendering refactor

[ashit-rath]

Description

This PR has core logic changes on how widgets are rendered. Instead of the entire DSL flowing through the flow of creating canvases and widgets, Only a structure similar to DSL is used to do the Same. This Structure only contains a bare bones structure, and the actual widget properties are selected directly from the store for each individual widgets.

Changes also includes,

• Updating state of changed widgets instead of all the widgets in the Redux state (using diff to find the updated widgets and also added to framework to skip diffing when updated widgets are already know, which can be used in the future on state updates).
• Selecting occupied spaces in components only when needed
• Replaced commentModeSelector which has expensive logic inside the selector itself that ran on every state change with a useCommentMode that only performs the calculation on when dependencies are changed.
• update selected widget state only when a needed instead of updating redux state on every action.

Fixes #12938

Type of change

• Performance improvement

How Has This Been Tested?

Manual UI and all existing tests should pass.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
appsmith	✅ Ready (Inspect)	Visit Preview	Aug 17, 2022 at 7:22AM (UTC)

[ashit-rath]

/ok-to-test sha=4d9cec6

[github-actions]

Unable to find test scripts. Please add necessary tests to the PR.

[github-actions]

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/2487274596.
Workflow: Appsmith External Integration Test Workflow.
Commit: 4d9cec6.
PR: 14485.

[ashit-rath]

/ok-to-test sha=8027618

[github-actions]

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/2487468931.
Workflow: Appsmith External Integration Test Workflow.
Commit: 8027618.
PR: 14485.

[ashit-rath]

/ok-to-test sha=28fb4fc

[riodeuno]

@rahulramesha Sounds good for now. In general, using deepEquals should be the last resort, as it is more of a patch fix in my opinion. Ideally, to prevent re-renders, the component data itself should have referential integrity. This applies to data used by react components in general.

Here, this means that instead of calling UPDATE_LAYOUT where we replace all values, we should ideally update individual paths which have changed. This would make the deepEquals moot.

[rahulramesha]

@riodeuno yes, but in this particular scenario, where we consume the state in only one component per page for the CanvasStructure that contains all the values, diffing logic to update specific paths would take up more time or be equal to the deepEqual.

But Ideally, in the future, when we decide to further replace children with just their IDs instead of whole tree structure of children, That would be a much better solution without deepEqual. But currently to do that will require more cleanup and there are more unknowns with respect to how few widgets work.

[rahulramesha]

/ok-to-test sha=60a9e24

[github-actions]

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/2839497897.
Workflow: Appsmith External Integration Test Workflow.
Commit: 60a9e24.
PR: 14485.

[github-actions]

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/2839497897.
Commit: 60a9e24.
Results:

Click to view performance test results

	Run 1	Run 2	Run 3	Run 4	Run 5	Median	Mean	SD.Sample	SD.Population
SELECT_WIDGET_MENU_OPEN
scripting	1214.09	1164.32	1125.46	1057.4	1093.65	1125.46	1130.98	5.39	4.82
painting	7.53	7.18	12.19	10.45	10.25	10.25	9.52	22.27	19.96
rendering	897.59	903.48	844.07	828.42	859.53	859.53	866.62	3.80	3.40
SELECT_WIDGET_SELECT_OPTION
scripting	145.44	155.27	139.16	142.32	136.09	142.32	143.66	5.13	4.59
painting	3.71	4.59	1.77	3.73	5.72	3.73	3.9	37.18	33.33
rendering	313.41	310.68	290.78	307.5	296.88	307.5	303.85	3.17	2.83

[rahulramesha]

/ok-to-test sha=e5d0eb7

[github-actions]

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/2867088601.
Workflow: Appsmith External Integration Test Workflow.
Commit: e5d0eb7.
PR: 14485.

[somangshu]

@rahulramesha there is a pending change request, Can you request the author for an approval or make the change if required.

@vivekverma2312 @laveena-en can you please give us an update on what is happening here with the QA regression.

[rahulramesha]

/ok-to-test sha=ddd5477

[rahulramesha]

@riodeuno, looks like there is change request still on the PR, since the review comments were all answered, could you please give a final review on the PR.

[github-actions]

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/2873656744.
Workflow: Appsmith External Integration Test Workflow.
Commit: ddd5477.
PR: 14485.

[vivekverma2312]

@somangshu Testing is in progress. 7 widgets are pending, will be done by today EOD.

[somangshu]

Awesome.

@rahulramesha we can merge this as soon as we have the signoff on the rest

[vivekverma2312]

@rahulramesha Completed regression on widgets and working fine.

[SatishGandham]

Looks good.
Congratulations @rahulramesha 🥳🥳🥳