pvaPy icon indicating copy to clipboard operation
pvaPy copied to clipboard

Review performance testing

Open ajgdls opened this issue 3 years ago • 10 comments

Hello, I've written a small Python module https://github.com/ajgdls/EPICSPyClientPerformance to compare the performance of 6 Python based EPICS clients. I have attempted to test each in a consistent way, and the README has some preliminary results for monitoring 1000 records at 10Hz for 1000 samples.

I would really appreciate it if this module owner would review EPICSPyClientPerformance test code for this client implementation to check that I'm actually testing in the correct and most efficient way. I would be happy to make updates that shows an increase in performance for this client.

This testing is driven by Diamond Light Source, and on 1st December the tests will be re-run against latest PyPI versions and then published (probably tech-talk) for further discussion, so if anyone wants to push performance optimisations they will get picked up on that date.

Thanks for taking a look, looking forward to hearing from everyone.

ajgdls avatar Nov 02 '22 15:11 ajgdls

Hi,

Thanks for doing this, I believe this is very useful. I just have few comments:

  1. The pvapy monitoring code can be simplified since you only have a single subscriber (only monitor/stopMonitor calls are needed). For example:
def echo(x):
    print('New PV value: %s' % x)
channel.monitor(echo, 'field(value,alarm,timeStamp)')

This will not affect performance results, however.

  1. APS has done some performance tests for pvapy and p4p that you might want to take a look at. The scripts and initial testing was done by Tom Fors (see https://git.aps.anl.gov/tfors/python-pva-test), and the most recent update by T. Guruswamy (https://git.aps.anl.gov/tguruswamy/python-pva-test). I am not sure if those repos and reports are public, but if you can't get to them let me know and we can get you access.

  2. A different set of tests were done for pvapy in order to measure its ability to handle images at very high rates. Results and tests are summarized here: https://github.com/epics-base/pvaPy/blob/master/documentation/streamingFramework.md#performance-testing

  3. In your test results, it would be useful to have a column describing how many PV updates were received vs missed. For higher rates you might need to include small server side queue to smooth out networking issues (for all clients).

Hope this helps, if you need more info, or would like access to APS git repos I mentioned, please let me know.

sveseli avatar Nov 02 '22 17:11 sveseli

Hi @sveseli thanks for you fast response. I will take a look through your comments.

ajgdls avatar Nov 03 '22 15:11 ajgdls

Hi @sveseli may I ask a quick question about the requestDescriptor parameter in the Channel.get method? I appear to get the same response (which I believe is a full structure) whether I have: requestDescriptor='field(value,alarm,timestamp)' or requestDescriptor='field(value)'

Am I misunderstanding this parameter? In both cases in my test the response I got was:

epics:nt/NTScalar:1.0 
    double value 4305
    alarm_t alarm
        int severity 1
        int status 1
        string message HIGH
    time_t timeStamp
        long secondsPastEpoch 1667994386
        int nanoseconds 39987581
        int userTag 0
    structure display
        double limitLow 0
        double limitHigh 0
        string description 
        string units 
        int precision 0
        enum_t form
            int index 0
            string[] choices [Default, String, Binary, Decimal, Hex, Exponential, Engineering]
    control_t control
        double limitLow 0
        double limitHigh 0
        double minStep 0
    valueAlarm_t valueAlarm
        boolean active false
        double lowAlarmLimit nan
        double lowWarningLimit nan
        double highWarningLimit 1
        double highAlarmLimit nan
        int lowAlarmSeverity 0
        int lowWarningSeverity 0
        int highWarningSeverity 0
        int highAlarmSeverity 0
        byte hysteresis 0

Thanks!

ajgdls avatar Nov 09 '22 15:11 ajgdls

You are correct as far as the parameter usage. Let me look into this and see what is going on.

sveseli avatar Nov 09 '22 15:11 sveseli

This is expected, the behavior depends on the specific PVA server that you're talking to, due to a disagreement several years ago between Marty Kraimer and Michael Davidsaver. Marty's original idea was that the server should only return exactly the list of fields you asked for in the pvRequest, but Michael's implementation of the QSRV server ignores that and sends everything (so it could add extra fields beyond those requested if it needed to).

anjohnson avatar Nov 09 '22 16:11 anjohnson

Thanks @anjohnson . I just checked the parameter operation against PvaServer (PvDatabase based server), and it works as expected, sending only fields that were requested.

sveseli avatar Nov 09 '22 16:11 sveseli

... Michael's implementation of the QSRV server ignores that and sends everything ...

To reiterate... The pvRequest mask is not ignored. Rather, it is interpreted differently by QSRV. The complete data structure is shown. However, only changes to selected fields are sent over the network. So for a field which is not selected, a client will see the definition (if it looks), but will not receive data updates.

mdavidsaver avatar Nov 09 '22 16:11 mdavidsaver

@ajgdls , here is how you can test this against PvaServer instance.

Terminal 1:

Type "help", "copyright", "credits" or "license" for more information.
>>> from pvapy import *
>>> nt = NtScalar(INT,10)
>>> s = PvaServer('nt', nt)
>>> 

Terminal 2:

$ python -c "from pvapy import *; c = Channel('nt'); pv = c.get(); print(pv)"
structure 
    int value 10

$ python -c "from pvapy import *; c = Channel('nt'); pv = c.get('field(value,alarm)'); print(pv)"
structure 
    int value 10
    alarm_t alarm
        int severity 0
        int status 0
        string message 

$ python -c "from pvapy import *; c = Channel('nt'); pv = c.get('field(value,alarm,timeStamp)'); print(pv)"
structure 
    int value 10
    alarm_t alarm
        int severity 0
        int status 0
        string message 
    time_t timeStamp
        long secondsPastEpoch 0
        int nanoseconds 0
        int userTag 0

sveseli avatar Nov 09 '22 16:11 sveseli

@mdavidsaver Thanks, I wasn't aware of the "only send changes to the fields" interpretation that QSRV takes for the field list.

anjohnson avatar Nov 09 '22 16:11 anjohnson

OK I understand, thanks all (and thanks @sveseli for the example you provide)

ajgdls avatar Nov 09 '22 17:11 ajgdls