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

Audio nodes: don't use atomics for interior mutability #350

Merged
merged 12 commits into from
Sep 21, 2023

Conversation

orottier
Copy link
Owner

@orottier orottier commented Jul 27, 2023

  • AnalyserNode
  • AudioBufferSourceNode
  • AudioDestinationNode
  • BiquadFilterNode
  • ChannelConfig
  • ChannelMergerNode
  • ChannelSplitterNode
  • ConstantSourceNode
  • ConvolverNode
  • DelayNode
  • DynamicsCompressorNode
  • GainNode
  • IIRFilterNode
  • MediaElementAudioSourceNode
  • MediaStreamAudioDestinationNode
  • MediaStreamAudioSourceNode
  • MediaStreamTrackAudioSourceNode
  • OscillatorNode
  • PannerNode
  • StereoPannerNode
  • WaveShaperNode

@b-ma
Copy link
Collaborator

b-ma commented Sep 5, 2023

Heiho,

How would you like to advance on that? Separate PRs for each node? pushing stuff to this one? Let me know, I can help if needed

@orottier
Copy link
Owner Author

orottier commented Sep 7, 2023

I added a monster commit to demo the swamp I am currently in. Will proceed shortly

@orottier orottier force-pushed the feature/no-interior-mutability-with-atomics branch from 8238e0b to fcdd80e Compare September 21, 2023 06:49
@orottier
Copy link
Owner Author

The previous commits were rather large because I have made the start/stop methods of AudioScheduledSourceNode take &mut. This was necessary to store some state in the AudioBufferSourceNode.

I think the rest of the work will move fast now. I will start from the top of the node list (in the top comment of this PR)
If you would like to help you could maybe move from the bottom up?

@b-ma
Copy link
Collaborator

b-ma commented Sep 21, 2023

If you would like to help you could maybe move from the bottom up?

Yup let's go like that, I'm a bit busy these days but I will try to advance when I find small time slots

@orottier
Copy link
Owner Author

Thanks for the contribution. Only the PannerNode is left which I will try to fix today (it's a bit more work)

@orottier
Copy link
Owner Author

Okay, I think this is ready to be merged.
Shall I release a new version first with the stable device-ids? And then merge this part so we can focus on the required node binding changes?

@orottier orottier marked this pull request as ready for review September 21, 2023 15:41
@b-ma
Copy link
Collaborator

b-ma commented Sep 21, 2023

Ok cool was quite fast indeed

Shall I release a new version first with the stable device-ids? And then merge this part so we can focus on the required node binding changes?

No, I don't think this is needed, don't bother with that

@orottier
Copy link
Owner Author

Alright!
/bench

@github-actions
Copy link

Benchmark result:


bench_ctor
  Instructions:             4559254 (-0.329164%)
  L1 Accesses:              6808595 (-0.330689%)
  L2 Accesses:                54341 (+0.018406%)
  RAM Accesses:               61548 (-0.006499%)
  Estimated Cycles:         9234480 (-0.245000%)

bench_sine
  Instructions:            71060357 (-0.157697%)
  L1 Accesses:            103667846 (-0.174783%)
  L2 Accesses:               263675 (+0.666593%)
  RAM Accesses:               62424 (+0.006408%)
  Estimated Cycles:       107171061 (-0.160830%)

bench_sine_gain
  Instructions:            76157984 (-0.225454%)
  L1 Accesses:            111349295 (-0.246741%)
  L2 Accesses:               270808 (-0.622741%)
  RAM Accesses:               62524 (No change)
  Estimated Cycles:       114891675 (-0.246501%)

bench_sine_gain_delay
  Instructions:           151285330 (-0.162967%)
  L1 Accesses:            214035861 (-0.199137%)
  L2 Accesses:               565964 (+5.611381%)
  RAM Accesses:               64141 (+0.009355%)
  Estimated Cycles:       219110616 (-0.126037%)

bench_buffer_src
  Instructions:            17639933 (-0.634494%)
  L1 Accesses:             25612100 (-0.692940%)
  L2 Accesses:                87324 (-1.707545%)
  RAM Accesses:              100713 (-0.011914%)
  Estimated Cycles:        29573675 (-0.627411%)

bench_buffer_src_delay
  Instructions:            91437957 (-0.257613%)
  L1 Accesses:            126499231 (-0.302033%)
  L2 Accesses:               166644 (+1.987185%)
  RAM Accesses:              100758 (-0.122916%)
  Estimated Cycles:       130858981 (-0.282962%)

bench_buffer_src_iir
  Instructions:            41806143 (-1.017087%)
  L1 Accesses:             60820183 (-0.303013%)
  L2 Accesses:                87778 (-2.149243%)
  RAM Accesses:              100800 (-0.021821%)
  Estimated Cycles:        64787073 (-0.300486%)

bench_buffer_src_biquad
  Instructions:            37849517 (-0.804872%)
  L1 Accesses:             53122799 (-0.923153%)
  L2 Accesses:               119634 (-5.724282%)
  RAM Accesses:              100947 (-0.006934%)
  Estimated Cycles:        57254114 (-0.919846%)

bench_stereo_positional
  Instructions:            45633756 (-1.744191%)
  L1 Accesses:             68007364 (-1.938461%)
  L2 Accesses:               274239 (+0.125961%)
  RAM Accesses:              101019 (-0.017815%)
  Estimated Cycles:        72914224 (-1.808923%)

bench_stereo_panning_automation
  Instructions:            32229577 (-1.518502%)
  L1 Accesses:             48123415 (-1.965162%)
  L2 Accesses:               138892 (-0.836760%)
  RAM Accesses:              100826 (-0.006942%)
  Estimated Cycles:        52346785 (-1.820723%)

bench_analyser_node
  Instructions:            39786501 (-0.320079%)
  L1 Accesses:             55678863 (-0.363450%)
  L2 Accesses:               182004 (+0.108357%)
  RAM Accesses:              101351 (-0.020716%)
  Estimated Cycles:        60136168 (-0.336189%)


@orottier orottier merged commit ed2e871 into main Sep 21, 2023
3 checks passed
@orottier orottier deleted the feature/no-interior-mutability-with-atomics branch September 21, 2023 18:02
@orottier
Copy link
Owner Author

Changing the ChannelConfig may still arrive out of order with other setting changes, I will try to draft something

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