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

Driver Station Performance Improvements #97

Merged
merged 2 commits into from
Jan 13, 2016

Conversation

ThadHouse
Copy link
Member

The old driver station code had some performance issues. It would cache the data under lock, which would take about 5ms. In addition, only 1 thread could read from the cache at one time, which could be a problem. So the code was changed to use a ReaderWriterLockSlim, as writes are called much less then reads. The second commit changes the code to use a cache to pull the data from the HAL, and then grabs the lock and does a reference change, which can happen much quicker.

Locking the joysticks now uses a ReaderWriterLock, and we now get the
descriptor in the GetData loop. This should speed up the DriverStation,
and make it more reliable.
The GetData HAL methods take 5 ms combined to grab all the data. So
there is easily a period where the data could be locked and contended
for. This now grabs the data into a cache, leave all joystick data
readable. It then grabs the write lock, does a quick reference swap, and
unlocks. This way, the  writing only takes a few nanoseconds, rather
then 5 milliseconds.

Should fix DS Joystick Tests

The tests need to wait longer.

Extends wait times

Might need to actually figure out locking driver station.
@codecov-io
Copy link

Current coverage is 35.62%

Merging #97 into master will increase coverage by +0.31% as of 977c40a

@@            master     #97   diff @@
======================================
  Files          121     121       
  Stmts         7884    7949    +65
  Branches       982     985     +3
  Methods          0       0       
======================================
+ Hit           2784    2832    +48
  Partial        146     146       
- Missed        4954    4971    +17

Review entire Coverage Diff as of 977c40a

Powered by Codecov. Updated on successful CI builds.

@ThadHouse
Copy link
Member Author

I pushed these fixes upstream as well. Don't know if they'll get merged, but they might. I saw performance improvements for sure, so I might merge this after a few days of testing.

@ThadHouse
Copy link
Member Author

Since this actually fixes bugs, I've done some testing and it works better. Merging, closing #98.

@ThadHouse ThadHouse closed this Jan 13, 2016
@ThadHouse ThadHouse reopened this Jan 13, 2016
ThadHouse added a commit that referenced this pull request Jan 13, 2016
Driver Station Performance Improvements
@ThadHouse ThadHouse merged commit 78a39d7 into master Jan 13, 2016
@jkoritzinsky jkoritzinsky deleted the DriverStationReadWriteLock branch March 28, 2016 15:09
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

Successfully merging this pull request may close these issues.

2 participants