pcdshub / typhos

Automatic-yet-customizable Graphical User Interface Generation for Ophyd Devices
http://pcdshub.github.io/typhos
Other
16 stars 26 forks source link

REF: lazy load all tools and hopefully improve performance #500

Closed klauer closed 2 years ago

klauer commented 2 years ago

Description

Motivation and Context

How Has This Been Tested?

Where Has This Been Documented?

This PR text

Screenshots (if appropriate):

klauer commented 2 years ago

There may be some bugs introduced - or made easier to hit - with this branch. Need to investigate:

$ devtyphos at2l0
[2022-05-26 11:59:51] - INFO - Loading 'at2l0' ...
[2022-05-26 11:59:55] - INFO - Loading Tools ...
[2022-05-26 11:59:56] - INFO - Adding devices ...
Segmentation fault
ZLLentz commented 2 years ago

We call load_best_template twice (due to a setattr in a property and then an explicit call)

This explains A LOT of the oddities I was seeing a while back while fixing other internals

ZLLentz commented 2 years ago

The code here looks good/normal- should I give this a test drive combined with your pydm PR?

klauer commented 2 years ago

The PyDM PR is only necessary if you want to run the profiler, of course. I think it'd be a good idea to have you take a look at as well, if you can spare the time 👍

I'd be curious if you can reproduce any startup core dumps, as now that I'm trying to cause them I'm failing (of course!).

ZLLentz commented 2 years ago

I'm not able to produce any issues at all with this so far This definitely makes the 1-device suites feel noticeably faster to load, the multi-device suites still feel pretty sluggish

ZLLentz commented 2 years ago

(Of course, I've been running with your pydm patch the entire time RE: startup core dumps)

klauer commented 2 years ago

Two additional commits here - relatively minor, and one could be removed after a discussion.

The --exit-after was added so I could test typhos initialization in a loop and try to get it to crash

ZLLentz commented 2 years ago

I think it's ok to leave --exit-after in- might replace it later if we get a better metric for when to close the screen for benchmarking