pcdshub / atef

PCDS Automated Test Execution Framework
https://pcdshub.github.io/atef/
Other
3 stars 10 forks source link

PERF/REF: Lazy Page loading #224

Closed tangkong closed 10 months ago

tangkong commented 10 months ago

Description

This is going to be a doozy. This has the simple goal of lazily loading PageWidgets, and the not-so-simple reality of rewriting most of said widgets.

A high-level list of things I did

Motivation and Context

(#222) One user programmatically generated a checkout with 3000 comparisons, and the GUI failed to load it. It attempted to, but way ATEF is structured every single widget had to be loaded.

Briefly, the previous functionality was:

To accomplish this we have to extricate the tree logic from the widgets.

A key requirement is that each widget must be loadable at any point by selecting its tree item.

Because widgets always existed, it was natural to tie the QDataclassBridges to the widgets. As such any update behavior/callbacks need to access widgets (that now may not exist). I think it makes the most sense here to move the bridges to the TreeItem, and base behavior on that. (this was quite disruptive)

How Has This Been Tested?

Test suite runs, but mostly interactively. Perhaps I need to expand the test suite more...

Where Has This Been Documented?

Verbosely in this PR, as I always do

Pre-merge checklist

klauer commented 10 months ago

Crash:

``` $ python -m atef config atef/tests/configs/active_test.json (click run; then click through the tree - and it crashes at SetValueStep) QThread: Destroyed while thread is still running Abort trap: 6 (atef) PC98125:atef klauer$ /Users/klauer/mc/envs/atef/lib/python3.9/multiprocessing/resource_tracker.py:216: UserWarning: resource_tracker: There appear to be 1 leaked semaphore objects to clean up at shutdown warnings.warn('resource_tracker: There appear to be %d ' ``` Repeatable on psbuild so I used gdb there: ``` (gdb) py-bt Traceback (most recent call first): File "/cds/home/k/klauer/Repos/atef/atef/widgets/config/window.py", line 626, in show_page_for_data new_widget = self.create_widget(item, mode) File "/cds/home/k/klauer/Repos/atef/atef/widgets/config/window.py", line 610, in show_selected_display ``` `thread apply all py-bt` marked this thread sitting off in libca land: ``` Thread 56 (Thread 0x7fff78a80700 (LWP 18193) "BusyCursorThrea"): Traceback (most recent call first): Waiting for the GIL File "/cds/group/pcds/pyps/conda/py39/envs/pcds-5.8.0/lib/python3.9/site-packages/epics/ca.py", line 910, in pend_event ret = libca.ca_pend_event(timeout) File "/cds/group/pcds/pyps/conda/py39/envs/pcds-5.8.0/lib/python3.9/site-packages/epics/ca.py", line 923, in poll pend_event(evt) File "/cds/group/pcds/pyps/conda/py39/envs/pcds-5.8.0/lib/python3.9/site-packages/epics/ca.py", line 547, in wrapper return fcn(*args, **kwds) File "/cds/group/pcds/pyps/conda/py39/envs/pcds-5.8.0/lib/python3.9/site-packages/epics/ca.py", line 1072, in connect_channel poll() File "/cds/group/pcds/pyps/conda/py39/envs/pcds-5.8.0/lib/python3.9/site-packages/epics/ca.py", line 577, in wrapper return fcn(*args, **kwds) File "/cds/group/pcds/pyps/conda/py39/envs/pcds-5.8.0/lib/python3.9/site-packages/epics/pv.py", line 440, in connect ca.connect_channel(self.chid, timeout=timeout) File "/cds/group/pcds/pyps/conda/py39/envs/pcds-5.8.0/lib/python3.9/site-packages/epics/pv.py", line 47, in wrapped return func(self, *args, **kwargs) File "/cds/group/pcds/pyps/conda/py39/envs/pcds-5.8.0/lib/python3.9/site-packages/epics/pv.py", line 425, in wait_for_connection self.connect(timeout=timeout) File "/cds/group/pcds/pyps/conda/py39/envs/pcds-5.8.0/lib/python3.9/site-packages/epics/pv.py", line 54, in wrapped return func(self, *args, **kwargs) File "/cds/group/pcds/pyps/conda/py39/envs/pcds-5.8.0/lib/python3.9/site-packages/ophyd/signal.py", line 1229, in _ensure_connected if not pv.wait_for_connection(timeout=timeout): File "/cds/group/pcds/pyps/conda/py39/envs/pcds-5.8.0/lib/python3.9/site-packages/ophyd/signal.py", line 1786, in wait_for_connection self._ensure_connected(self._read_pv, self._write_pv, timeout=timeout) File "/cds/home/k/klauer/Repos/atef/atef/widgets/config/data_active.py", line 937, in get_curr_value self._sig.wait_for_connection() File "/cds/home/k/klauer/Repos/atef/atef/widgets/utils.py", line 202, in run self.func() ```
tangkong commented 10 months ago

RE: crash

I went through earlier and replaced functions-defined-in-functions with WeakPartialMethodSlot, but didn't do the same for functions passed to QThreads. I wonder if making those functions proper methods would help there.

tangkong commented 10 months ago

A realization about Ken's crash: the segfault is happening in an Edit widget, while SetValueStep has as unique run-widget. The edit-widget should not be invoked. (Past me left a TODO I see...)

Edit: fixing the logic removes the crash. Basically the edit widget was created then orphaned, while a specific Run PageWidget was created and displayed instead. That orphaned widget got cleaned up while ophyd threads were running.

Interesting I didn't run into this before, but forward progress is progress

tangkong commented 10 months ago

After some "memory profiling" with top and the huge config, PVConfigurationPage's with many sub-comparisons still take prohibitively long to load, likely due to having to load all the row widgets in the comparison table. This will be an issue for other ConfigurationGorup pages, and is seen in other apps like the pmps-ui.

External to the effort, investigating paginated table layouts is worthwhile. Alternately, using native Qt widgets may be helpfule, but prevent us from doing all these snazzy things

tangkong commented 10 months ago

Thanks everyone. Shipping this off