Skip to content

dramatic speed improvements and code cleanup#69

Open
Neywiny wants to merge 4 commits intomhostetter:maint-3.10from
Neywiny:dev
Open

dramatic speed improvements and code cleanup#69
Neywiny wants to merge 4 commits intomhostetter:maint-3.10from
Neywiny:dev

Conversation

@Neywiny
Copy link

@Neywiny Neywiny commented Sep 1, 2025

Mostly things I noticed in #68. Includes some things that were needed to run on a modern Debian Trixie system, including converting to float before going into the PMT and fixing deprecated functionality like nan and datetime utc functions.

I recorded a file off my RTL_SDR of the mag^2 such that the the only blocks are the file source and blocks from this repo.

Test compiled flowgraph
#!/usr/bin/env python3
# -*- coding: utf-8 -*-

#
# SPDX-License-Identifier: GPL-3.0
#
# GNU Radio Python Flow Graph
# Title: ADS-B Receiver
# Author: Matt Hostetter
# GNU Radio version: 3.10.9.2

from gnuradio import blocks
import pmt
from gnuradio import gr
from gnuradio.filter import firdes
from gnuradio.fft import window
import sys
import signal
from argparse import ArgumentParser
from gnuradio.eng_arg import eng_float, intx
from gnuradio import eng_notation
import gnuradio.adsb as adsb




class adsb_rx(gr.top_block):

    def __init__(self):
        gr.top_block.__init__(self, "ADS-B Receiver", catch_exceptions=True)

        ##################################################
        # Variables
        ##################################################
        self.threshold = threshold = 0.1
        self.fs = fs = 2e6
        self.fc = fc = 1090e6

        ##################################################
        # Blocks
        ##################################################

        self.blocks_null_sink_0 = blocks.null_sink(gr.sizeof_float*1)
        self.blocks_file_source_0 = blocks.file_source(gr.sizeof_float*1, 'mags.bin', False, 0, 0)
        self.blocks_file_source_0.set_begin_tag(pmt.PMT_NIL)
        self.adsb_framer_1 = adsb.framer(fs, threshold)
        self.adsb_demod_0 = adsb.demod(fs)
        self.adsb_decoder_0 = adsb.decoder("Extended Squitter Only", "None", "Brief")


        ##################################################
        # Connections
        ##################################################
        self.msg_connect((self.adsb_demod_0, 'demodulated'), (self.adsb_decoder_0, 'demodulated'))
        self.connect((self.adsb_demod_0, 0), (self.blocks_null_sink_0, 0))
        self.connect((self.adsb_framer_1, 0), (self.adsb_demod_0, 0))
        self.connect((self.blocks_file_source_0, 0), (self.adsb_framer_1, 0))


    def get_threshold(self):
        return self.threshold

    def set_threshold(self, threshold):
        self.threshold = threshold
        self.adsb_framer_1.set_threshold(self.threshold)

    def get_fs(self):
        return self.fs

    def set_fs(self, fs):
        self.fs = fs

    def get_fc(self):
        return self.fc

    def set_fc(self, fc):
        self.fc = fc




def main(top_block_cls=adsb_rx, options=None):
    tb = top_block_cls()

    def sig_handler(sig=None, frame=None):
        tb.stop()
        tb.wait()

        sys.exit(0)

    signal.signal(signal.SIGINT, sig_handler)
    signal.signal(signal.SIGTERM, sig_handler)

    tb.start()

    tb.wait()


if __name__ == '__main__':
    main()

The file is ~1 GB at 2 Msps, so ~ 2 ish minutes. Within that time period I saw a few dozen messages. Not as many as I'd like, but it's something. Running the existing code (+ the nan and PMT float change to get it to at least run) I see:

Benchmark 1: ./adsb_rx.py
  Time (mean ± σ):     60.484 s ±  0.716 s    [User: 31.072 s, System: 34.338 s]
  Range (min … max):   59.325 s … 61.327 s    10 runs

With my code:

Benchmark 1: ./adsb_rx.py
  Time (mean ± σ):     47.305 s ±  0.677 s    [User: 25.369 s, System: 25.957 s]
  Range (min … max):   45.964 s … 48.438 s    10 runs

I did not have the file stored in a ramdisk or anything, but I had run it a few times so any caching should be the same for both. I also did my best to not be doing anything else on the computer at the same time. There were a few temporary microbenchmarks run for prove-outs of what was faster vs slower.

I tried to not change functionality but in my opinion the timestamping in the output table was incorrect. It should show the last time the aircraft was seen, not be the same time for all rows. Likewise, I left the if Falses, because I know that while yes if you know the code was there you can go back in the git history, but sometimes it's nice for debug to just turn them on. However, at least one case there was compute being done that was only used in an if False so I just moved that in. On the same lines, there were a decent amount of times that variables were being assigned to the instance but only used within the same function, such as aa and aa_bits, and a few times the __init__ dealt with variables that were no longer around.

I caught one semicolon, so I don't know exactly where this code all came from, but I also tried to make it a bit more pythonic. There's certainly some python-isms that reduce performance (such as enums vs string comparison) that seemed out of scope and the goal is to speed things up.

Cloc reports:

Language files blank comment code
Existing 3 317 409 1105
This dev effort 3 307 403 1058

Considering this is ~80% the compute time and gets rid of 50 lines of code (less code means less maintenance, higher readability, etc etc), I think it's worthwhile to be investigated by the powers that be. I didn't see much in the way of tests, but I am willing to attempt getting a reproducible run between the two to make sure they decode everything identically.

@mhostetter
Copy link
Owner

Thanks for this. Give a me a bit to review and then we can get it merged.

@Neywiny
Copy link
Author

Neywiny commented Sep 2, 2025

Thanks for taking a look. I was thinking about it more today and realized the preamble array was still a Python integer list. Changing it to match the pulses list gets me down another few seconds.

# first checking how the computer is feeling today, seems about the same as last time. Maybe a tad slower
$ hyperfine ./adsb_rx.py --output=pipe
Benchmark 1: ./adsb_rx.py
  Time (mean ± σ):     61.330 s ±  0.922 s    [User: 31.587 s, System: 34.331 s]
  Range (min … max):   59.733 s … 63.046 s    10 runs

$ hyperfine ./adsb_rx.py --output=pipe
Benchmark 1: ./adsb_rx.py
  Time (mean ± σ):     45.577 s ±  0.596 s    [User: 22.630 s, System: 26.911 s]
  Range (min … max):   44.385 s … 46.727 s    10 runs

@Neywiny
Copy link
Author

Neywiny commented Sep 7, 2025

Another few seconds gone.

$ hyperfine ./adsb_rx.py --output=pipe
Benchmark 1: ./adsb_rx.py
  Time (mean ± σ):     41.554 s ±  0.973 s    [User: 21.434 s, System: 23.290 s]
  Range (min … max):   40.347 s … 43.642 s    10 runs

This code is now running ~2/3 the amount of time as the before these commits. I'm still working through the exact mechanics of how this correlation is working, but I am pretty sure this is a safe change to make.

Copy link

@peternmuller peternmuller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got the following error with your change:

/usr/local/lib/python3.12/dist-packages/gnuradio/adsb/framer.py:50: FutureWarning: In the future `np.bool` will be defined as the corresponding NumPy scalar.
  self.preamble_pulses = np.array([1,0,1,0,0,0,0,1,0,1,0,0,0,0,0,0], dtype=np.bool)
Traceback (most recent call last):
  File "/home/peter/gr-adsb/examples/adsb_rx.py", line 296, in <module>
    main()
  File "/home/peter/gr-adsb/examples/adsb_rx.py", line 273, in main
    tb = top_block_cls()
         ^^^^^^^^^^^^^^^
  File "/home/peter/gr-adsb/examples/adsb_rx.py", line 207, in __init__
    self.adsb_framer_1 = adsb.framer(fs, threshold)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/dist-packages/gnuradio/adsb/framer.py", line 50, in __init__
    self.preamble_pulses = np.array([1,0,1,0,0,0,0,1,0,1,0,0,0,0,0,0], dtype=np.bool)
                                                                             ^^^^^^^
  File "/usr/lib/python3/dist-packages/numpy/__init__.py", line 324, in __getattr__
    raise AttributeError(__former_attrs__[attr])
AttributeError: module 'numpy' has no attribute 'bool'.
`np.bool` was a deprecated alias for the builtin `bool`. To avoid this error in existing code, use `bool` by itself. Doing this will not modify any behavior and is safe. If you specifically wanted the numpy scalar type, use `np.bool_` here.
The aliases was originally deprecated in NumPy 1.20; for more details and guidance see the original release note at:
    https://numpy.org/devdocs/release/1.20.0-notes.html#deprecations. Did you mean: 'bool_'?

data type should be simply bool and not np.bool.

@Neywiny
Copy link
Author

Neywiny commented Oct 5, 2025

@peternmuller

Good catch. Looks like something's different from our installs where that's an error. I re-ran my benchmark with just bool and the average went up a fraction of a second (within variance), but the bounds got tighter (not sure if within variance). So, to me, these are functionally the same just like the note from NumPy said they would be. Changing to int8 was a tad slower which matches my initial development findings.

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