lmms icon indicating copy to clipboard operation
lmms copied to clipboard

Lb302 cleanup

Open rubiefawn opened this issue 1 month ago • 2 comments

Makes the following changes to the Lb302 plugin for increased maintainability and readability:

  • [x] Use non-static data member initializers where possible
  • [x] Use std::numbers::pi instead of M_PI
  • [x] Use std::array and std::unique_ptr instead of manual memory management
  • [x] Change plain-old-data classes with only public members to structs
  • [x] Change some ints to f_cnt_t, fpp_t, etc. to better communicate their purpose and avoid weird implicit casting nonsense
  • [x] Change floats to sample_t where appropriate to better communicate their purpose
  • [x] Find suitable homes for loose constants
  • [x] Prefix standard library math function calls with std::
  • [x] Ensure all floating-point literals intended to be floats end in f and are not doubles to appease MSVC
  • [x] Change uses of SIGNAL and SLOT macros to whatever they should be post-Qt6 upgrade
  • [x] Remove cursed mutex in favor of a real-time safe queue (TODO: possible generalization for #6516?)
  • [ ] Optimize struct/class member layout for better locality
  • [x] Completely conform to current code conventions

The above list is not comprehensive and is destined to expand.

rubiefawn avatar Nov 13 '25 09:11 rubiefawn

Figured I'd run some un-rigorous perf stat benchmarks of release builds as a sanity check to make sure I wasn't destroying performance by changing the mutex for a bunch of atomic operations and busy-wait loops.

Updated for commit f788be70d3b0cc3e27e5e6cb67c432ffa0102cef

Before, with mutex/dynamically allocated vector:

Performance counter stats for './lmms render /home/fawn/Documents/lmms/projects/torture.mmpz -a -f flac':

   103,084,983,781      task-clock                       #    2.979 CPUs utilized             
           695,800      context-switches                 #    6.750 K/sec                     
               388      cpu-migrations                   #    3.764 /sec                      
             6,173      page-faults                      #   59.883 /sec                      
   267,717,430,944      instructions                     #    1.17  insn per cycle            
   229,289,575,609      cycles                           #    2.224 GHz                       
    61,653,273,269      branches                         #  598.082 M/sec                     
       496,591,833      branch-misses                    #    0.81% of all branches           

      34.602878156 seconds time elapsed

      93.334079000 seconds user
       9.660192000 seconds sys

After, with atomic ringbuffer/statically allocated array:

Performance counter stats for './lmms render /home/fawn/Documents/lmms/projects/torture.mmpz -a -f flac':

   104,963,456,247      task-clock                       #    3.066 CPUs utilized             
           708,920      context-switches                 #    6.754 K/sec                     
                67      cpu-migrations                   #    0.638 /sec                      
             4,237      page-faults                      #   40.366 /sec                      
   267,503,252,259      instructions                     #    1.15  insn per cycle            
   233,160,958,544      cycles                           #    2.221 GHz                       
    61,493,266,240      branches                         #  585.854 M/sec                     
       511,104,882      branch-misses                    #    0.83% of all branches           

      34.239441489 seconds time elapsed

      95.604870000 seconds user
       9.199340000 seconds sys

Project file used for said benchmarks: torture.mmp.xml

rubiefawn avatar Dec 08 '25 08:12 rubiefawn

CACHELINE and busy_wait_hint probably don't belong inside Lb302. Where is the best place for me to move these to?

rubiefawn avatar Dec 08 '25 22:12 rubiefawn