Performance enhancements

[AlexanderWells-diamond]

This branch contains a number of performance enhancements for Coniql. This is a 20-25% decrease in CPU usage, which effectively increases Coniql's requests-per-second from ~1,000 to ~2,000, with larger performance gains for 5+ clients subscribing to the same set of PVs.

The largest performance gains have been from simply removing the async keyword wherever possible. Additional gains have been made by creating a connection pool in the CA Plugin, which means we only have 1 camonitor per PV, regardless of the number of client subscriptions for that PV.

There is also a fair amount of work enhancing the Python performance tests in here, although those changes do not affect Coniql's performance.

There is a new document titled "Performance", which goes into some details about the work done and where time is spent. Please let me know if this should have more details added.

Also of note is dropping Python 3.7 support. It looks like Strawberry, as of 0.197.0, is only Python 3.8+. I figure we want to keep up to date with Strawberry so this change seems inevitable.

Closes #86 (by deleting that code)

NOTE: I haven't written any tests for this, but I wanted to put the PR up for the actual code to get some more eyes on before I write any.

[AlexanderWells-diamond]

I've made commit https://github.com/DiamondLightSource/coniql/pull/94/commits/f4d794c82d6ec4583d9073266b53692238014426 to suppress the MyPy errors, and raised #95 to cover the work to resolve that.

[codecov[bot]]

Codecov Report

Merging #94 (3a8c46e) into main (86054d9) will increase coverage by 0.85%. Report is 11 commits behind head on main. The diff coverage is 95.91%.

@@            Coverage Diff             @@
##             main      #94      +/-   ##
==========================================
+ Coverage   93.30%   94.15%   +0.85%     
==========================================
  Files          10       10              
  Lines         807      856      +49     
==========================================
+ Hits          753      806      +53     
+ Misses         54       50       -4     

Files Changed	Coverage Δ
src/coniql/simplugin.py	94.54% <66.66%> (-0.52%)	:arrow_down:
src/coniql/types.py	94.11% <92.30%> (-0.62%)	:arrow_down:
src/coniql/caplugin.py	97.31% <95.32%> (+4.71%)	:arrow_up:
src/coniql/strawberry_schema.py	98.68% <98.59%> (+0.18%)	:arrow_up:
src/coniql/app.py	69.84% <100.00%> (+0.98%)	:arrow_up:

... and 1 file with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[rjwills28]

I've tested on my local system and also see the performance improvements.

I like the idea of only maintaining one camonitor per PV when there are multiple clients however I have come across a problem with this implementation. If I load a screen or send a subscription from the graphiql interface I see the value of that PV update. However, if I then open a second instance of the screen or send the same subscription from another graphiql instance I see that the value does not get displayed. The only way to get a value to display in this second instance is for that PV itself to get updated (i.e. caput xxx). Not a problem when the PV is updating at a given rate but otherwise I think this could be a problem. Let me know if you need any more details on how to reproduce.

[AlexanderWells-diamond]

Thanks for investigating that. I'm a little surprised at that though - line 262 of caplug.py exists to cover this situation. I wonder if it's something to do with the GraphiQL interface. Would you mind repeating the test, but instead of opening a new tab open an entirely new (private) window for the second connection?

[rjwills28]

I retested as described and also tried displaying 2 instances of a screen in cs-web-proto subscribing to a PV that is not on a scan update. I got the same result that the second screen/graphiql did not update. Note if both screens are open when Coniql starts up then all is OK. They key is starting Coniql and then opening one screen followed by the other. I added a debug statement before line 262 and saw that this was not printed when the second instance was loaded so I don't think it's even reaching this bit of the code.

[AlexanderWells-diamond]

Thanks again, I've clearly missed something. I'll investigate further.

[AlexanderWells-diamond]

I believe I've fixed this in https://github.com/DiamondLightSource/coniql/pull/94/commits/301398378890dc581d4217cdf5e832abfd63951a. It worked in local tests with non-updating and slowly updating PVs.

[AlexanderWells-diamond]

Thank you for the review. I plan to add tests to this PR before merging it.