AudioParam: Reduce code duplication and Arc usage

[uklotzde]

• Extract common fields into AudioParamBase
• Extract atomic fields into AudioParamShared, wrapped into a single Arc

The code might not be elegant. But revealing and making those dependencies explicit is the first step before improving the design.

The static decomposition should not have any noticeable impact on performance or memory usage, although the compiler now has less options for reordering and packing the fields. Using only a single Arc instead of two could improve performance slightly.

[github-actions]

Benchmark result:


bench_ctor
  Instructions:             5134276 (-0.064330%)
  L1 Accesses:              7699964 (-0.061210%)
  L2 Accesses:                54115 (-0.007391%)
  RAM Accesses:               61564 (-0.012993%)
  Estimated Cycles:        10125279 (-0.049515%)

bench_sine
  Instructions:            78179934 (-0.005142%)
  L1 Accesses:            115127660 (-0.001266%)
  L2 Accesses:               270052 (-1.565894%)
  RAM Accesses:               62659 (+0.006384%)
  Estimated Cycles:       118670985 (-0.019207%)

bench_sine_gain
  Instructions:            82926111 (-0.013983%)
  L1 Accesses:            122323299 (-0.014600%)
  L2 Accesses:               280095 (+1.620294%)
  RAM Accesses:               62810 (+0.004777%)
  Estimated Cycles:       125922124 (+0.003632%)

bench_sine_gain_delay
  Instructions:           156982259 (-0.007655%)
  L1 Accesses:            223431801 (-0.015552%)
  L2 Accesses:               747283 (+2.859994%)
  RAM Accesses:               64498 (No change)
  Estimated Cycles:       229425646 (+0.030143%)

bench_buffer_src
  Instructions:            18570684 (-0.021109%)
  L1 Accesses:             26915878 (-0.018848%)
  L2 Accesses:                91961 (-0.498799%)
  RAM Accesses:               96108 (+0.007284%)
  Estimated Cycles:        30739463 (-0.023203%)

bench_buffer_src_delay
  Instructions:            91612524 (+0.002317%)
  L1 Accesses:            126780823 (+0.013601%)
  L2 Accesses:               198285 (-7.812151%)
  RAM Accesses:               96298 (No change)
  Estimated Cycles:       131142678 (-0.050891%)

bench_buffer_src_iir
  Instructions:            42554705 (-0.009251%)
  L1 Accesses:             61855580 (-0.009261%)
  L2 Accesses:               100472 (+0.177478%)
  RAM Accesses:               96201 (+0.001040%)
  Estimated Cycles:        65724975 (-0.007309%)

bench_buffer_src_biquad
  Instructions:            38991229 (-0.014381%)
  L1 Accesses:             54952789 (+0.052327%)
  L2 Accesses:               141691 (-20.53002%)
  RAM Accesses:               96315 (+0.004153%)
  Estimated Cycles:        59032269 (-0.260431%)

bench_stereo_positional
  Instructions:            46303113 (-0.012935%)
  L1 Accesses:             68674010 (+0.117973%)
  L2 Accesses:               677336 (-11.65313%)
  RAM Accesses:               96486 (-0.001036%)
  Estimated Cycles:        75437700 (-0.482595%)

bench_stereo_panning_automation
  Instructions:            32662790 (-0.012603%)
  L1 Accesses:             48703543 (-0.005977%)
  L2 Accesses:               150681 (-1.885699%)
  RAM Accesses:               96232 (+0.002078%)
  Estimated Cycles:        52825068 (-0.032779%)

bench_analyser_node
  Instructions:            39116067 (-0.010734%)
  L1 Accesses:             54860927 (-0.003485%)
  L2 Accesses:               190011 (-2.123287%)
  RAM Accesses:               96800 (+0.004132%)
  Estimated Cycles:        59198982 (-0.037794%)

[orottier]

Thanks again.

The explicit layout of AudioParam > AudioParamRaw > AudioParamBase > AudioParamShared has some merits but snippets like this are rather daunting:

        self.raw_parts
            .base_parts
            .shared_parts
            .current_value
            .store(clamped, Ordering::SeqCst);

But revealing and making those dependencies explicit is the first step before improving the design.

Do you have any specific ideas or issues to address? It would help me decide whether this new layout pulls its weight

[uklotzde]

Do you have any specific ideas or issues to address? It would help me decide whether this new layout pulls its weight

These chains only appear internally and the actual memory layout is not visible from the outside. Reducing the redundancies upfront might be helpful when sorting out the synchronization between AudioParam and AudioParamProcessor as proposed in #297. Sure, they are not elegant. But that's a consequence of the current design which does not seem to be ideal.

I prefer to group multiple fields that appear repeatedly in different contexts as distinct types. Replicating them anonymously across different types would hide these dependencies. Makes it easier to spot commonalities and differences for subsequent refactorings. We should end up with very few types that share only a single, common core or base.

[orottier]

Thanks for the elaborate reply.

current design which does not seem to be ideal.
subsequent refactorings

My question about specific ideas or issues to address still stands. I'm hesitant to refactor for the sake of abstract reasons such as redundancies and commonalities if there is no pressing issue at hand. Surely, you are technically correct about the flat layout hiding possible future improvements, but to refactor also incurs a slight cost since the original authors of the file will have to grok the changes.

It would really help me make a judgement if you could share the next steps you have in mind with this module.

[uklotzde]

The issues I tried to fix are probably only the tip of the iceberg.