Chief Delphi

Chief Delphi (http://www.chiefdelphi.com/forums/index.php)
-   Programming (http://www.chiefdelphi.com/forums/forumdisplay.php?f=51)
-   -   NetworkTables needs to change. (http://www.chiefdelphi.com/forums/showthread.php?t=138318)

Jaci 28-09-2015 01:43

NetworkTables needs to change.
 
As I've discussed here, I feel that NetworkTables needs to change. Not only is it inefficient on the network, it's inefficient on the processor, both of which are royalties when we're talking about the RoboRIO and FRC's field bandwidth limit. I'd like to see everyone else's opinions on NetworkTables, and see if we can get it to change before next year's game.

Regards,
~Jaci R

Thad House 28-09-2015 03:07

Re: NetworkTables needs to change.
 
Quote:

Originally Posted by Jaci (Post 1497766)
As I've discussed here, I feel that NetworkTables needs to change. Not only is it inefficient on the network, it's inefficient on the processor, both of which are royalties when we're talking about the RoboRIO and FRC's field bandwidth limit. I'd like to see everyone else's opinions on NetworkTables, and see if we can get it to change before next year's game.

Regards,
~Jaci R

It has actually had a complete rewrite this summer. There is now a C++ library that all languages will use as the backend, instead of different implementations for the 3 official and many unofficial languages. It should be much better now.

I read through your blog as well. One thing is it doesn't send all messages every 100ms. It only sends a keep alive flag, and then any entries that have been updated.

In addition, there's a good reason why writes are limited to 100ms on a separate clock. Back in 2012, when the first implementation of NetworkTables was out, if I recall correctly there was no write limit, and it would write data as soon as you entered it to the table. This caused many more issues with lag, because teams would use SampleRobot, run the teleop loop as fast as possible, and then write to the SmartDashboard, which would bring code to a halt. By writing to a local store, and only updating the clients every 100 ms, you make sure even in that situation the robot will not start lagging. The time to write a new entry to the local store is orders of magnitude faster then writing to the network, so holding back data is the write option.

Anything written for FRC needs to be designed for teams with no programming experience or mentors to use. For many teams 100ms update times for the dashboard is more then fast enough, and if that helps reduce network and cpu load, which can make up for inefficient team code, its the far better choice.

Some of the powerhouse teams in FIRST do write their own communication code that runs faster, but those are also the teams that have the mentorship to do this. For 99% of FRC teams, you need something as simple and easy to use as NetworkTables and SmartDashboard.

Also, Cameras are a much bigger killer to the network then NetworkTables could even hope to be. At events, I've seen teams run probably 50 SmartDashboard variables, plus the Driver Station, and this only uses about 1.5 MBps of bandwith, whereas even a 320x240 camera uses close to 2 MBps.

One more thing is that NetworkTables really isn't designed for control packets, and shouldn't be. It should be designed for sending information. Robot control data like Joysticks through the DS are not sent using NT, and are heavily prioritized by the FMS and field networking system.

Jaci 28-09-2015 06:49

Re: NetworkTables needs to change.
 
Thanks for the reply, you raise good points.

For the smart dashboard implementation, I agree that the constant updating of data would be superfluous, however, I believe that this limit should be placed on the Dashboard itself instead of the NetworkTables instance, which would keep teams from spamming the pipeline as well as allowing them to send data as they need to.

I'm aware of teams using their own Communication Code, as I did the same thing this 2015 season. Sure, it solves some of the inefficiencies of NetworkTables, but the point I'm trying to make is you shouldn't have to write your own Comms code to make it efficient, if FRC wants to set a good example for coding, they should practice what they preach. Efficiency and Simplicity aren't mutually exclusive.

Furthermore, I'd like to see the codebase for NetworkTables cut down. 62 source files is a fair amount for the JVM to load (for Java teams, esp on the small amount of ram on the RIO), but you stating that there is a C++ library backend makes me hopeful this won't be as much of an issue in the future.

Thanks for pointing that out about the control code, I admit that in retrospect I made a mistake regarding control code in my article.

virtuald 28-09-2015 12:00

Re: NetworkTables needs to change.
 
I agree that NetworkTables has been a problem for awhile, and that the code quality isn't what it should have been -- I should know, I've spent a ton of time in the code, and have found/fixed a number of deadlocks and race conditions in it. However, the ease of use it provides is great, and the tools that have been created around it are quite useful.

I haven't seen the rewritten code yet, but I believe the one doing it is my Python co-collaborator and I have great faith that it'll get done right this time.

I strongly disagree that it should be moved to UDP. There's a reason TCP exists -- namely, it takes care of retransmission/etc for you, so you can depend (ish) that the data you sent will eventually get there (with lots of caveats on this, of course). Coupled with an transmit-on-update-only scheme like NetworkTables uses, this works out great, particularly for large tables or for variables that don't get updated often (which for our usage, is over half of them).

In UDP, if a packet gets dropped, then you have to either implement the retransmission yourself, or you need to ensure that the data gets transmitted often enough that it doesn't matter that you lose the packets.

Off the top of my head, there are two ways you can deal with this in UDP. Send all the data in every packet (which for large tables, would be bad) or ensure that the data gets sent often (which is still bad, doesn't adequately deal with the retransmit problem, and for things that don't get updated often like boolean flags, is annoying to implement). Neither of these sound like attractive options to me.

The internal 100ms (I set it to 50ms on python) wait of NetworkTables is not necessarily a *performance* problem, it's a latency problem. And honestly, that amount of latency doesn't matter 99% of the time if you design your code appropriately. In the cases that it does matter (like the DS packets), then yes, this scheme doesn't work. It would be useful to have a low-latency option built in (maybe by using TCP_NODELAY properly), but I suspect it would be overused by teams.

One other reason that the poll loop exists is that it's very important to decouple network latency from your robot's main loop. Transmitting a packet on the network (even in UDP, particularly on the cRio, not so much on the RoboRIO) can take a non-trivial amount of time compared to the rest of things that you do in your robot's main loop -- particularly if your wireless connection has interference in it -- so sending the packets directly from your robot's main loop is a bad idea. They chose to decouple this with a separate transmit thread and a queue, but it could certainly be implemented more efficiently with an epoll-like event-based mechanism if desired. In fact, many of the performance/deadlock problems NT had initially (and, are still there to some degree) are related to improper locking that led to the main loop waiting for network packets to transmit.

Thad House 28-09-2015 12:26

Re: NetworkTables needs to change.
 
Quote:

Originally Posted by Jaci (Post 1497772)
Furthermore, I'd like to see the codebase for NetworkTables cut down. 62 source files is a fair amount for the JVM to load (for Java teams, esp on the small amount of ram on the RIO), but you stating that there is a C++ library backend makes me hopeful this won't be as much of an issue in the future.

Have you guys ever had issues with ram on the RIO? I ran some test code in multiple languages for hours, and unless I explicitly put in a memory leak, I could never use more then like 20MB of ram. With 256MB total, there is more then enough.

virtuald 28-09-2015 12:52

Re: NetworkTables needs to change.
 
Quote:

Originally Posted by Thad House (Post 1497805)
Have you guys ever had issues with ram on the RIO? I ran some test code in multiple languages for hours, and unless I explicitly put in a memory leak, I could never use more then like 20MB of ram. With 256MB total, there is more then enough.

The only time I've ran into RAM issues was when trying to compile OpenCV on the RoboRIO itself.

Alan Anderson 28-09-2015 13:29

Re: NetworkTables needs to change.
 
Quote:

Originally Posted by Jaci (Post 1497766)
...I feel that NetworkTables needs to change.

Quote:

Originally Posted by Thad House (Post 1497770)
It has actually had a complete rewrite this summer.

How's that for service? :cool:

calcmogul 28-09-2015 14:05

Re: NetworkTables needs to change.
 
Quote:

Originally Posted by virtuald (Post 1497800)
I haven't seen the rewritten code yet, but I believe the one doing it is my Python co-collaborator and I have great faith that it'll get done right this time.

The repository for the new NetworkTables code is located at https://github.com/PeterJohnson/ntcore. It's included in WPILib as a Git submodule.

JesseK 28-09-2015 15:12

Re: NetworkTables needs to change.
 
Quote:

Originally Posted by mathmogul (Post 1497820)
The repository for the new NetworkTables code is located at https://github.com/PeterJohnson/ntcore. It's included in WPILib as a Git submodule.

Do you know if there is a min / max supported Java version with this? i.e. Java 8 only?

calcmogul 28-09-2015 15:22

Re: NetworkTables needs to change.
 
Quote:

Originally Posted by JesseK (Post 1497852)
Do you know if there is a min / max supported Java version with this? i.e. Java 8 only?

I'm not sure. WPILib uses Java 8 exclusively so that's all we've bothered to test. Peter could give a definitive answer.

Peter Johnson 28-09-2015 17:46

Re: NetworkTables needs to change.
 
Quote:

Originally Posted by mathmogul (Post 1497857)
I'm not sure. WPILib uses Java 8 exclusively so that's all we've bothered to test. Peter could give a definitive answer.

I'll try to respond to some of the other messages in this thread later this evening, but first, a couple of quick notes.

I know of only one definite Java 8 dependency in the new NT, and I just committed it a couple days ago: I added more advanced notification features but I didn't want to break backwards compatibility with existing Java code, so I added a "default" implementation of the new interface function to ITableListener. Having a "default" like this is unfortunately a Java 8 feature, but this should be easy to work around for previous versions if you don't care about backwards compatibility. Is there real interest out there for older Java versions?

That being said, I've not actually tested it with older Java versions, so it's possible there's another dependency, but the new Java code is a thin wrapper around a JNI interface to the C++ library, so there's only 5 or 6 Java files (and only one real "implementation" class).

Java and C++ will both use the new C++ implementation, but it's still up in the air whether LabView will use the C++ library or continue using a LabView-native implementation (with the same protocol improvements).

Please note there are a few API changes in the library that required updates to wpilib code, which means it won't work "out of the box" with last year's wpilib (the C++ version I know has API breakage; I need to double-check, but the Java API should have minimal to no breakage).

It's worth noting the new C++ implementation is interoperable with old clients and servers, with the only downside being you won't have access to the new features from the older clients/servers (of course).

JesseK 29-09-2015 10:02

Re: NetworkTables needs to change.
 
I don't have an interest in older Java versions - just wanted to know what to look for when profiling the JNI stuff.

Thad House 29-09-2015 12:45

Re: NetworkTables needs to change.
 
Quote:

Originally Posted by Peter Johnson (Post 1497905)
Java and C++ will both use the new C++ implementation, but it's still up in the air whether LabView will use the C++ library or continue using a LabView-native implementation (with the same protocol improvements).

I would hope that LabVIEW chooses to go with the new library. Having only 1 version of the communication protocol makes debugging MUCH easier. It's also not that hard to port, and would probably only take a day or 2 if somebody works on it full time.

1 other great change this allows is adding new languages easily. Since all the comms code is in the native library, as long as your language can somehow interop into the C library, all you need is a very thin wrapper to gain full functionality. When I ported the old NetworkTables to C#, it was not easy, especially dealing with the endian issues. Porting the new library was much much easier, took less then a day, and you only had to write interop code and not communication code.

Tom Line 29-09-2015 19:23

Re: NetworkTables needs to change.
 
Quote:

Originally Posted by Thad House (Post 1498013)
I would hope that LabVIEW chooses to go with the new library. Having only 1 version of the communication protocol makes debugging MUCH easier. It's also not that hard to port, and would probably only take a day or 2 if somebody works on it full time.

1 other great change this allows is adding new languages easily. Since all the comms code is in the native library, as long as your language can somehow interop into the C library, all you need is a very thin wrapper to gain full functionality. When I ported the old NetworkTables to C#, it was not easy, especially dealing with the endian issues. Porting the new library was much much easier, took less then a day, and you only had to write interop code and not communication code.

I'm going to second this. We had a real headache with network tables this year. The driver station received data just fine, but trying to send data to the robot was hit-and-miss.

fovea1959 02-10-2015 15:39

Re: NetworkTables needs to change.
 
Quote:

Originally Posted by virtuald (Post 1497800)
I haven't seen the rewritten code yet, but I believe the one doing it is my Python co-collaborator and I have great faith that it'll get done right this time.

Is there a strong possibility that pynetworktables2js will have appropriate changes made to keep working? I'm thinking about having the team move to that for dashboards....


All times are GMT -5. The time now is 05:01.

Powered by vBulletin® Version 3.6.4
Copyright ©2000 - 2017, Jelsoft Enterprises Ltd.
Copyright © Chief Delphi