search

jisotalo / ads-client

Unofficial Node.js ADS library for connecting to Beckhoff TwinCAT automation systems using ADS protocol.
https://jisotalo.fi/ads-client/
MIT License
77 stars 18 forks source link

Performance issue of parsing code #57

Closed martin-gawlita closed 3 years ago

martin-gawlita commented 3 years ago

Hi,

I'm currently evaluating your project for usage in the software of my company. Impressive work! Your library is well documented.

While using it I came to a performance issue when subscribing to an array of 100 LREAL values. After some time the subscription gets lost because the mailbox of the TwinCAT router gets overfilled. I've profiled the code and have seen that there is a very time consuming operation running everytime data is send or received via ADS which you'll find in the following code block:

https://github.com/jisotalo/ads-client/blob/a06849d200e23ce8ff26ab674d07ca24cb27e474/src/ads-client-ads.js#L883-L890

It seems that you look for the type of the read or written symbol everytime an ADS request is made or received to be able to parse it. I've seen approaches where this task was solved in a less time consuming manner. Is there a reason you've explicitly chose this way?

Best,

Martin

jisotalo commented 3 years ago

Hi Martin!

Thanks for the feedback, glad to hear!

I checked yesterday the code and indeed it can be done a little faster. We can first search for the data type, if it's not found, we can use RegExp if it's string or array. However, I have never had any performance issues.

What kind of code are you using and how small is your cycle time when subscribing?

I will look into that code, no worries! But still wondering if it will help.

martin-gawlita commented 3 years ago

Hi!

thank you for your quick response.

This sounds like a good idea. I've done a performance test to be honest. In this test I'm subscribing to an array of 1000 LREAL values with a cycle time of 1 ms. In parallel I'm reading and writing an array of 100 LREAL values every 20 ms. Shortly after starting the test the TwinCAT Router Mailbox gets overfilled.

Best,

Martin

jisotalo commented 3 years ago

Yeah that 1ms cycle time is way too small interval in my opinion. I would suggest to have at least 10ms or 100ms interval. If you really need data every 1ms could you collect many "1ms packages" at PLC and send them together? The 1ms cycle time is very fast even in realtime side of the PLC.

Similar problems here but that time the TCP window got full too: https://github.com/jisotalo/ads-client/issues/48

martin-gawlita commented 3 years ago

The cycle time of 1 ms is very small, you are right. With the C++ and .NET Library of Beckhoff this is possible so I though I might give it a try. It seems that the TCP/IP Interface of Beckhoff or the TCP/IP stack of node isn't as quick as the native Libraries of Beckhoff.

jisotalo commented 3 years ago

Hi! I just released 1.10.5 where that code part is optimized a little. Please open a new issue if there are problems.

Changes: https://github.com/jisotalo/ads-client/commit/876e4ccac2c0db2fefd19cb4d7bdc50abf3f37e5#diff-e77f22b17ed16819b4585f2a1c6c2ea8cb3c3d7e369fc019ea874b6cb9cba268