Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review performance testing #78

Open
ajgdls opened this issue Nov 2, 2022 · 10 comments
Open

Review performance testing #78

ajgdls opened this issue Nov 2, 2022 · 10 comments

Comments

@ajgdls
Copy link

ajgdls commented Nov 2, 2022

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.

@sveseli
Copy link
Collaborator

sveseli commented Nov 2, 2022

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.

@ajgdls
Copy link
Author

ajgdls commented Nov 3, 2022

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

@ajgdls
Copy link
Author

ajgdls commented Nov 9, 2022

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!

@sveseli
Copy link
Collaborator

sveseli commented Nov 9, 2022

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

@anjohnson
Copy link
Member

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).

@sveseli
Copy link
Collaborator

sveseli commented Nov 9, 2022

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

@mdavidsaver
Copy link
Member

... 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.

@sveseli
Copy link
Collaborator

sveseli commented Nov 9, 2022

@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

@anjohnson
Copy link
Member

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

@ajgdls
Copy link
Author

ajgdls commented Nov 9, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants