Skip to content

UAVCAN footprint reduction#3005

Merged
LorenzMeier merged 18 commits intomasterfrom
uavcan_footprint_reduction
Nov 13, 2015
Merged

UAVCAN footprint reduction#3005
LorenzMeier merged 18 commits intomasterfrom
uavcan_footprint_reduction

Conversation

@pavel-kirienko
Copy link
Copy Markdown
Member

@LorenzMeier @davids5 @bendyer @mhkabir

This optimization feels like cheating.

ROM reduction (I swear I didn't touch the compiler flags): 980204 - 967772 = 12 K.
RAM reduction: it's complicated.

nsh> uavcan status
uavcan: application not running
nsh> free
             total       used       free    largest
Mem:        229760     147360      82400      78800
nsh> uavcan start
uavcan: Node ID 1, bitrate 1000000
uavcan: SW version vcs_commit: 0x588a7afa
uavcan: sensor bridge 'gnss' init ok
uavcan: sensor bridge 'mag' init ok
uavcan: sensor bridge 'baro' init ok
nsh> free
             total       used       free    largest
Mem:        229760     158304      71456      66544
nsh> uavcan start fw
uavcan fw srv: base: couldn't open /etc/uavcan/fw
nsh> free
             total       used       free    largest
Mem:        229760     170272      59488      52064

Connecting Zubax GNSS, which requires the driver to sponge more memory, yet we can see that fragmentation does not increase (pay attention to the field largest):

nsh> uavcan_mag adding channel 124...
uavcan_mag channel 124 class instance 1 ok
uavcan: GNSS receiver node ID: 124
free
             total       used       free    largest
Mem:        229760     172112      57648      52064

Connecting another node (px4cannode-v1) with an outdated firmware - the driver initiates the update procedure; at the end of it we see this:

nsh> free
             total       used       free    largest
Mem:        229760     173840      55920      52064

In this particular test, worst case memory use was 82400 - 55920 = 26K.

What happened?

  • The driver used to maintain multiple (3 exactly) independent block pools for different tasks. This approach is wildly inefficient. Now the driver uses one huge allocator that is thread-safe and therefore can serve everything.
  • The old allocator used to allocate memory from an internal pool implemented as a class field. Therefore the driver had to allocate huge continuous chunks of memory, causing fragmentation and sadness. The new allocator uses a linked list of chunks that grows dynamically as memory needs increase, taking memory from the heap in 64-byte chunks (mm overhead included). Once a chunk has been allocated, the allocator keeps it for future use, therefore the driver doesn't malloc()/free() constantly (actually it never free()s anything on its own).
  • The new allocator is limited in growth, and the limit can be runtime adjusted. The limit is currently 500 blocks, or 32000 bytes.
  • It is possible to shrink() memory reserved by the allocator. The driver will penalize you for that with higher CPU usage for the following few allocations, because it will have to malloc() again. Possible usage scenario: shrinking before arming to provide more memory for log buffers.
  • The transport layer and some other stuff in the library has been changed also to avoid static allocation in favor of block allocation. It's complicated and I don't want to go into details here, the old solution was really not very efficient.

Stay tuned, work is still in progress.

@pavel-kirienko pavel-kirienko self-assigned this Oct 16, 2015
@pavel-kirienko
Copy link
Copy Markdown
Member Author

@pavel-kirienko
Copy link
Copy Markdown
Member Author

According to OpenCyphal-Garage/libcyphal#33, RAM reduction in the case above is about 42K - 26K = 16K.

@pavel-kirienko
Copy link
Copy Markdown
Member Author

CPU load during firmware update at 1 Mbps:

 186 uavcan                       8269 10.105  1508/ 1792 240 (240)  w:sem 
 197 uavcan fw srv                5471  6.259  3556/ 5992 120 (120)  w:sem 

Testing with firmware update and some nodes connected shows that peak memory usage does not exceed 80 blocks, so the limit of 500 blocks seems enough.

@LorenzMeier
Copy link
Copy Markdown
Member

Nice!

@pavel-kirienko pavel-kirienko force-pushed the uavcan_footprint_reduction branch from 5430a54 to f872cbf Compare October 16, 2015 21:57
@pavel-kirienko
Copy link
Copy Markdown
Member Author

@bendyer @mhkabir Does anybody want to give it a try? Seems to work flawlessly for me so far, although it still leaks about ~300 bytes of memory per restart (@davids5 master does too). The problem is certainly not with libuavcan (it's unit tested for leaks), could be either the application, some part of PX4 or NuttX. I'm not sure how important this issue is, considering that the driver is not supposed to be stopped during normal operation.

@pavel-kirienko
Copy link
Copy Markdown
Member Author

Seems like I've fixed the leaks. It appears to be ready for merge now.

@bendyer @mhkabir so would you like to test it, or shall we merge it as is?

@LorenzMeier
Copy link
Copy Markdown
Member

@pavel-kirienko A test flight with this change would be great.

@LorenzMeier LorenzMeier reopened this Oct 18, 2015
@LorenzMeier
Copy link
Copy Markdown
Member

Toggling to re-run hans-ci.

@LorenzMeier LorenzMeier reopened this Oct 18, 2015
@LorenzMeier LorenzMeier force-pushed the uavcan_footprint_reduction branch from 9a90bce to d90192d Compare October 19, 2015 07:49
@LorenzMeier
Copy link
Copy Markdown
Member

@pavel-kirienko @bendyer Is this flight tested? I'll rebase it on master to make it easier to handle.

@LorenzMeier LorenzMeier force-pushed the uavcan_footprint_reduction branch from d90192d to de452c9 Compare October 26, 2015 08:45
@pavel-kirienko
Copy link
Copy Markdown
Member Author

Not yet. Some hardware needed for the test is still stuck in post. I'll update you once it's tested.

@LorenzMeier LorenzMeier force-pushed the uavcan_footprint_reduction branch from de452c9 to 96a12a6 Compare October 26, 2015 09:20
@LorenzMeier
Copy link
Copy Markdown
Member

Good to go for a test flight.

@pavel-kirienko
Copy link
Copy Markdown
Member Author

I'm going to test fly this early next week. Should resolve #3102.

@pavel-kirienko
Copy link
Copy Markdown
Member Author

This has been test flown on an Iris with UAVCAN ESC without issues.

@mhkabir
Copy link
Copy Markdown
Member

mhkabir commented Nov 12, 2015

I test flew this today as well. System loading / memory looks good! :)

@pavel-kirienko
Copy link
Copy Markdown
Member Author

Will someone volunteer to merge this? I have already updated this PR to the yesterday's master.

LorenzMeier added a commit that referenced this pull request Nov 13, 2015
@LorenzMeier LorenzMeier merged commit 4f4f1d9 into master Nov 13, 2015
@LorenzMeier LorenzMeier deleted the uavcan_footprint_reduction branch November 13, 2015 18:34
davids5 pushed a commit that referenced this pull request Jan 12, 2017
  We simply call the srrink methode after the server stop.
  See #3005 (comment)
  for backgound
davids5 pushed a commit that referenced this pull request Jan 12, 2017
* Update libuavcan to upstream master with PX4 contrib for NuttX 7.16+

* Release any 64B blocks not needed by usavcan after FW server is stopped.

  We simply call the srrink methode after the server stop.
  See #3005 (comment)
  for backgound
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.

3 participants