Initialize view range and render time charts during trace indexing #1089
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I tried this PR with a large trace. When I opened the trace, then the overview view shows an overlay "Analysis Running". When opening a view I see the same message. I'm not able to change the time range while the analysis is running. So, I'm not sure how to test this change and verify the changed behaviour. Could you please describe how to test? If you have a video that you can share would also help. |
PatrickTasse
force-pushed
the
master
branch
2 times, most recently
from
553794a
to
303d8d9
Compare
bhufmann
reviewed
Aug 14, 2024
There was a problem hiding this comment.
I'm testing your patch that allows for continuous update of views. When having a time graph view open and do some random zooming (with right mouse button and drag) and time selection (with left mouse button and drag) I get the following exception and the trace viewer panel looses all graphs. It doesn't happen in master because I can't zoom/select during analysis running:
react-dom.development.js:27292 Uncaught Error: Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops.
at checkForNestedUpdates (react-dom.development.js:27292:1)
at scheduleUpdateOnFiber (react-dom.development.js:25475:1)
at dispatchSetState (react-dom.development.js:17527:1)
at onEmpty (FormControl.js:185:1)
at InputBase.js:305:1
at InputBase.js:310:1
at commitHookEffectListMount (react-dom.development.js:23150:1)
at commitLayoutEffectOnFiber (react-dom.development.js:23273:1)
at commitLayoutMountEffects_complete (react-dom.development.js:24688:1)
at commitLayoutEffects_begin (react-dom.development.js:24674:1)
checkForNestedUpdates @ react-dom.development.js:27292
scheduleUpdateOnFiber @ react-dom.development.js:25475
dispatchSetState @ react-dom.development.js:17527
onEmpty @ FormControl.js:185
(anonymous) @ InputBase.js:305
(anonymous) @ InputBase.js:310
commitHookEffectListMount @ react-dom.development.js:23150
commitLayoutEffectOnFiber @ react-dom.development.js:23273
commitLayoutMountEffects_complete @ react-dom.development.js:24688
commitLayoutEffects_begin @ react-dom.development.js:24674
commitLayoutEffects @ react-dom.development.js:24612
commitRootImpl @ react-dom.development.js:26823
commitRoot @ react-dom.development.js:26682
performSyncWorkOnRoot @ react-dom.development.js:26117
flushSyncCallbacks @ react-dom.development.js:12042
(anonymous) @ react-dom.development.js:25651
Show 16 more frames
Show less
logger-protocol.ts:117 2024-08-13T19:20:08.608Z root ERROR The above error occurred in the <ForwardRef(InputBase)> component:
at InputBase (http://localhost:3000/vendors-node_modules_fortawesome_free-solid-svg-icons_index_es_js-node_modules_fortawesome_re-8690be.js:34222:83)
at OutlinedInput (http://localhost:3000/vendors-node_modules_fortawesome_free-solid-svg-icons_index_es_js-node_modules_fortawesome_re-8690be.js:36965:82)
at div
at http://localhost:3000/vendors-node_modules_fortawesome_free-solid-svg-icons_index_es_js-node_modules_fortawesome_re-8690be.js:18064:66
at FormControl (http://localhost:3000/vendors-node_modules_fortawesome_free-solid-svg-icons_index_es_js-node_modules_fortawesome_re-8690be.js:32236:82)
at http://localhost:3000/vendors-node_modules_fortawesome_free-solid-svg-icons_index_es_js-node_modules_fortawesome_re-8690be.js:18064:66
at TextField (http://localhost:3000/vendors-node_modules_fortawesome_free-solid-svg-icons_index_es_js-node_modules_fortawesome_re-8690be.js:39404:83)
at div
at div
at div
at div
at div
at div
at TimegraphOutputComponent (http://localhost:3000/node_modules_moment_locale_sync_recursive_-theia-extensions_viewer-prototype_lib_browser_trac-d9389d.js:5889:9)
at Resizable (http://localhost:3000/vendors-node_modules_fortawesome_free-solid-svg-icons_index_es_js-node_modules_fortawesome_re-8690be.js:233505:35)
at DraggableCore (http://localhost:3000/vendors-node_modules_fortawesome_free-solid-svg-icons_index_es_js-node_modules_fortawesome_re-8690be.js:227111:5)
at GridItem (http://localhost:3000/vendors-node_modules_fortawesome_free-solid-svg-icons_index_es_js-node_modules_fortawesome_re-8690be.js:228080:5)
at div
at ReactGridLayout (http://localhost:3000/vendors-node_modules_fortawesome_free-solid-svg-icons_index_es_js-node_modules_fortawesome_re-8690be.js:228774:5)
at ResponsiveReactGridLayout (http://localhost:3000/vendors-node_modules_fortawesome_free-solid-svg-icons_index_es_js-node_modules_fortawesome_re-8690be.js:229788:5)
at WidthProvider (http://localhost:3000/vendors-node_modules_fortawesome_free-solid-svg-icons_index_es_js-node_modules_fortawesome_re-8690be.js:230328:7)
at div
at div
at TraceContextComponent (http://localhost:3000/node_modules_moment_locale_sync_recursive_-theia-extensions_viewer-prototype_lib_browser_trac-d9389d.js:7272:9)
at div
Consider adding an error boundary to your tree to customize error handling behavior.
Visit https://reactjs.org/link/error-boundaries to learn more about error boundaries.
I'm currently unable to explain this exception, but found that we can prevent it by waiting for analysis completion before rendering the search bar. |
bhufmann
requested changes
Aug 20, 2024
There was a problem hiding this comment.
The error still happens with a different flavor (callstack).
Uncaught Error Error: Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops.
at checkForNestedUpdates (/home/eedbhu/git/theia-training/theia-trace-extension/node_modules/react-dom/cjs/react-dom.development.js:27292:1)
at scheduleUpdateOnFiber (/home/eedbhu/git/theia-training/theia-trace-extension/node_modules/react-dom/cjs/react-dom.development.js:25475:1)
at enqueueSetState (/home/eedbhu/git/theia-training/theia-trace-extension/node_modules/react-dom/cjs/react-dom.development.js:14067:1)
at Component.setState (/home/eedbhu/git/theia-training/theia-trace-extension/node_modules/react/cjs/react.development.js:354:1)
at TimegraphOutputComponent.onSelectionChange (/home/eedbhu/git/theia-training/theia-trace-extension/packages/react-components/src/components/timegraph-output-component.tsx:1453:14)
at <anonymous> (/home/eedbhu/git/theia-training/theia-trace-extension/node_modules/timeline-chart/src/time-graph-row-controller.ts:24:58)
at TimeGraphRowController.handleSelectedRowChanged (/home/eedbhu/git/theia-training/theia-trace-extension/node_modules/timeline-chart/src/time-graph-row-controller.ts:24:45)
at set (/home/eedbhu/git/theia-training/theia-trace-extension/node_modules/timeline-chart/src/time-graph-row-controller.ts:103:14)
at <anonymous> (/home/eedbhu/git/theia-training/theia-trace-extension/node_modules/timeline-chart/src/layer/time-graph-chart.ts:483:52)
at step (localhost꞉3000/vendors-node_modules_fortawesome_free-solid-svg-icons_index_es_js-node_modules_fortawesome_re-8690be.js:253806:23)
at <anonymous> (localhost꞉3000/vendors-node_modules_fortawesome_free-solid-svg-icons_index_es_js-node_modules_fortawesome_re-8690be.js:253787:53)
at <anonymous> (localhost꞉3000/vendors-node_modules_fortawesome_free-solid-svg-icons_index_es_js-node_modules_fortawesome_re-8690be.js:253781:71)
at push.../../node_modules/timeline-chart/lib/layer/time-graph-chart.js.__awaiter (localhost꞉3000/vendors-node_modules_fortawesome_free-solid-svg-icons_index_es_js-node_modules_fortawesome_re-8690be.js:253777:12)
at push.../../node_modules/timeline-chart/lib/layer/time-graph-chart.js.TimeGraphChart.maybeFetchNewData (localhost꞉3000/vendors-node_modules_fortawesome_free-solid-svg-icons_index_es_js-node_modules_fortawesome_re-8690be.js:254227:16)
at push.../../node_modules/timeline-chart/lib/layer/time-graph-chart.js.TimeGraphChart.updateChart (/home/eedbhu/git/theia-training/theia-trace-extension/node_modules/timeline-chart/src/layer/time-graph-chart.ts:387:18)
at componentDidUpdate (/home/eedbhu/git/theia-training/theia-trace-extension/packages/react-components/src/components/timegraph-output-component.tsx:337:33)
at commitLayoutEffectOnFiber (/home/eedbhu/git/theia-training/theia-trace-extension/node_modules/react-dom/cjs/react-dom.development.js:23333:1)
at commitLayoutMountEffects_complete (/home/eedbhu/git/theia-training/theia-trace-extension/node_modules/react-dom/cjs/react-dom.development.js:24688:1)
at commitLayoutEffects_begin (/home/eedbhu/git/theia-training/theia-trace-extension/node_modules/react-dom/cjs/react-dom.development.js:24674:1)
at commitLayoutEffects (/home/eedbhu/git/theia-training/theia-trace-extension/node_modules/react-dom/cjs/react-dom.development.js:24612:1)
at commitRootImpl (/home/eedbhu/git/theia-training/theia-trace-extension/node_modules/react-dom/cjs/react-dom.development.js:26823:1)
at commitRoot (/home/eedbhu/git/theia-training/theia-trace-extension/node_modules/react-dom/cjs/react-dom.development.js:26682:1)
at performSyncWorkOnRoot (/home/eedbhu/git/theia-training/theia-trace-extension/node_modules/react-dom/cjs/react-dom.development.js:26117:1)
at flushSyncCallbacks (/home/eedbhu/git/theia-training/theia-trace-extension/node_modules/react-dom/cjs/react-dom.development.js:12042:1)
at <anonymous> (/home/eedbhu/git/theia-training/theia-trace-extension/node_modules/react-dom/cjs/react-dom.development.js:25651:1)
Looking at the callstack it shows that setState is called from componentDidUpdate as it is stated in the error message:
TimeGraphOutputComponent.componentDidUpdate() -> TimeGrpahChart.updateChart() -> TimeGraphChart.maybeFetchNewData() ->... -> TimeGraphRowController.handleSelectedRowChanged() -> TimeGraphOutputCompnent.onSelectionChange() -> Component.setState()
PatrickTasse
force-pushed
the
master
branch
4 times, most recently
from
eda2d50
to
90be8ec
Compare
PatrickTasse
changed the title
|
The solution has been changed to initialize the view range only once, to prevent slowdown due to constant refresh of the time charts. This PR should be used with the following changes in timeline-chart: |
bhufmann
previously approved these changes
Sep 20, 2024
There was a problem hiding this comment.
It looks good to me. I think it's a good idea to not continuously and repeatedly fetch the timeline chart states during running of the analysis. This would slow down the backend if the view also queries repeatedly the backend during analysis. Showing a fixed range (current available view) make sense and let user zoom in or out manually.
Thanks for the update.
Initialize the unit controller view range with either the trace full range, or the persisted state view range if it is set. When the experiment hasn't been indexed yet, the initial view range may be empty. When a new output is opened while the trace is indexing, and the view range has never been set yet, initialize the view range to the current partial trace full range. This view range will remain during and after indexing unless the user changes the view range manually or presses the Reset button. When indexing completes, if the view range has never been set yet, initialize the view range to the final trace full range. In TimegraphOutputComponent, render the timegraph content and use the analysis-running-overflow overlay while the analysis is running. Let the analysis-running-overflow pass through pointer events to the underlying layer. In TimegraphOutputComponent componentDidUpdate(), update the chart layers only if the outputStatus has changed, not always when the outputStatus is RUNNING. This prevents recursive change of state causing stack overflow exceptions. Update the chart layers also if the timegraphTree state has changed. Perform a 'soft' update that does not refresh rows that already have the correct data. Perform a 'hard' update only where the marker categories or marker set have changed. Prevent a possible double update of the markers chart layer. Extract search bar code to private method and fix typo in its className. Use a different id for the markers chart layer to help debugging. Fix AbstractXYOutputComponent checkedSeries array comparison. Signed-off-by: Patrick Tasse <[email protected]>
PatrickTasse
force-pushed
the
master
branch
from
90be8ec
to
d883dbe
Compare
bhufmann
approved these changes
Sep 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Initialize the unit controller view range with either the trace full
range, or the persisted state view range if it is set. When the
experiment hasn't been indexed yet, the initial view range may be empty.
When a new output is opened while the trace is indexing, and the view
range has never been set yet, initialize the view range to the current
partial trace full range. This view range will remain during and after
indexing unless the user changes the view range manually or presses the
Reset button.
When indexing completes, if the view range has never been set yet,
initialize the view range to the final trace full range.
In TimegraphOutputComponent, render the timegraph content and use the
analysis-running-overflow overlay while the analysis is running.
Let the analysis-running-overflow pass through pointer events to the
underlying layer.
In TimegraphOutputComponent componentDidUpdate(), update the chart
layers only if the outputStatus has changed, not always when the
outputStatus is RUNNING. This prevents recursive change of state causing
stack overflow exceptions. Update the chart layers also if the
timegraphTree state has changed. Perform a 'soft' update that does not
refresh rows that already have the correct data. Perform a 'hard' update
only where the marker categories or marker set have changed. Prevent a
possible double update of the markers chart layer.
Extract search bar code to private method and fix typo in its className.
Use a different id for the markers chart layer to help debugging.