Asset and track copy data multiple times about audio HOT 24 CLOSED

I'll get a reproduction example up asap. In the mean time here is some basic profile output:

2020/08/04 23:30:33 create mixer - Diff: 0 MiB, Alloc: 0 MiB, TotalAlloc: 0 MiB, Sys: 68 MiB, NumGC: 0
2020/08/04 23:30:36 Add track 0 - Diff: 183 MiB, Alloc: 184 MiB, TotalAlloc: 11386 MiB, Sys: 467 MiB, NumGC: 115
2020/08/04 23:30:38 Add track 1 - Diff: 347 MiB, Alloc: 532 MiB, TotalAlloc: 22770 MiB, Sys: 732 MiB, NumGC: 162
2020/08/04 23:30:40 Add track 2 - Diff: 112 MiB, Alloc: 644 MiB, TotalAlloc: 34154 MiB, Sys: 1062 MiB, NumGC: 194
2020/08/04 23:30:50 Add track 3 - Diff: 181 MiB, Alloc: 826 MiB, TotalAlloc: 78187 MiB, Sys: 1988 MiB, NumGC: 263
2020/08/04 23:30:59 Add track 4 - Diff: 269 MiB, Alloc: 1095 MiB, TotalAlloc: 122220 MiB, Sys: 2584 MiB, NumGC: 312
2020/08/04 23:31:09 Add track 5 - Diff: 881 MiB, Alloc: 1977 MiB, TotalAlloc: 166252 MiB, Sys: 3113 MiB, NumGC: 350
2020/08/04 23:31:18 Add track 6 - Diff: 258 MiB, Alloc: 2235 MiB, TotalAlloc: 210284 MiB, Sys: 3708 MiB, NumGC: 382
2020/08/04 23:31:28 Add track 7 - Diff: 616 MiB, Alloc: 2851 MiB, TotalAlloc: 254316 MiB, Sys: 4170 MiB, NumGC: 409
2020/08/04 23:31:28 pre-mix - Diff: 0 MiB, Alloc: 2851 MiB, TotalAlloc: 254316 MiB, Sys: 4170 MiB, NumGC: 409
2020/08/04 23:31:32 mix complete - Diff: 277 MiB, Alloc: 3129 MiB, TotalAlloc: 254593 MiB, Sys: 4174 MiB, NumGC: 409
2020/08/04 23:31:32 Pipe took 58.496539146s

from audio.

Hello @haswalt! Really glad to hear that you want to use it! I assume that you are using pipelined.dev/pipe v0.7.x, could you confirm please? That version indeed used extensive allocations in both components (mixer & asset) and pipe implementations. There were the following issues in the pipe repo: pipelined/pipe#86 and pipelined/pipe#84.

The fix for pipe has been already released as part of v.0.8.0. The support of latest pipe implementation for components in pipelined.dev/audio is already in master, but it wasn't released yet. Let me know if you want to test this implementation as well, I can help you with migration.

from audio.

Hi @dudk I’m actually testing out master because of those same issues. Also because there are some breakages in the way the mods load.

1 place I noticed an allocation is when using AddClip. Could be reduced by converting asset to an interface and implement it in the wav, mp3, flax packages and use / reuse those instances directly to avoid copying into the link struct. Would also make adding to lines or tracks much simpler when working with real files as currently asset seems to be any extra building block that doesn’t really add much

from audio.

I’m trying to get a simple example up today but I think the mod paths are broken in master as the wave package is a sub path of audio so can’t found

from audio.

@haswalt you are correct, new vanity imports were not pushed yet. I fixed it now: master branch of audio/wav, audio/flac and audio/mp3 should be available for go get. Let me know if it's still broken for you.

In the latest version of AddClip asset is not used anymore and everything is based on signal package interfaces. It avoids allocating new buffers and creates only references to slices of signal data.

from audio.

@dudk Managed to get a demo up and running, thank you for pushing those changes up:

https://github.com/haswalt/pipelined

Currently looks like the major culprit is actually loading the wav file into an asset. The demo isn't very efficient in that it loads. anew asset even though it's the same file but this is just to demonstrate the problem.

I'm not 100% sure as haven't fully dug into it yet but I think it's to do with the way the wav.Source is routed into the asset.

Assuming I'm not doing anything wrong in the code?

from audio.

I've created a really simple test for comparison. First test just opens the file, copys to a byte slice then closes the file:

2020/08/05 13:06:49 Start - Diff: 0 MiB, Alloc: 0 MiB, TotalAlloc: 0 MiB, Sys: 67 MiB, NumGC: 0
2020/08/05 13:06:49 Open file - Diff: 0 MiB, Alloc: 0 MiB, TotalAlloc: 0 MiB, Sys: 67 MiB, NumGC: 0
2020/08/05 13:06:49 Read file 1322298 - Diff: 3 MiB, Alloc: 4 MiB, TotalAlloc: 4 MiB, Sys: 69 MiB, NumGC: 1
2020/08/05 13:06:49 complete took 3.158274ms

Second test then opens the same file and routes it into an asset via the wav decoder:

2020/08/05 13:07:04 Start - Diff: 0 MiB, Alloc: 0 MiB, TotalAlloc: 0 MiB, Sys: 67 MiB, NumGC: 0
2020/08/05 13:07:04 Open file - Diff: 0 MiB, Alloc: 0 MiB, TotalAlloc: 0 MiB, Sys: 67 MiB, NumGC: 0
2020/08/05 13:07:04 Create source - Diff: 0 MiB, Alloc: 0 MiB, TotalAlloc: 0 MiB, Sys: 67 MiB, NumGC: 0
2020/08/05 13:07:04 Create line - Diff: 0 MiB, Alloc: 0 MiB, TotalAlloc: 0 MiB, Sys: 67 MiB, NumGC: 0
2020/08/05 13:07:04 Asset copied - Diff: 15 MiB, Alloc: 15 MiB, TotalAlloc: 1656 MiB, Sys: 70 MiB, NumGC: 229
2020/08/05 13:07:04 complete took 486.033694m

The second results are as expected, no change in memory until the pipe actually runs since nothing happens with anything except create pointers.

Here is a test simply to show the wav decoder running with no other functionality:

2020/08/05 13:18:55 Start - Diff: 0 MiB, Alloc: 0 MiB, TotalAlloc: 0 MiB, Sys: 67 MiB, NumGC: 0
2020/08/05 13:18:55 Open file - Diff: 0 MiB, Alloc: 0 MiB, TotalAlloc: 0 MiB, Sys: 67 MiB, NumGC: 0
2020/08/05 13:18:55 Init decoder - Diff: 0 MiB, Alloc: 0 MiB, TotalAlloc: 0 MiB, Sys: 67 MiB, NumGC: 0
2020/08/05 13:18:55 Create buffer - Diff: 0 MiB, Alloc: 0 MiB, TotalAlloc: 0 MiB, Sys: 67 MiB, NumGC: 0
2020/08/05 13:18:55 Load data -395792 - Diff: 1 MiB, Alloc: 1 MiB, TotalAlloc: 1 MiB, Sys: 69 MiB, NumGC: 0
2020/08/05 13:18:55 complete took 12.66279ms

from audio.

Here's the heap from my contrived example
heap.out.zip

from audio.

Thank you for the example project @haswalt! I did profiling on it and it confirms your finding: extensive allocations happen during asset.Sink:

pipelined.dev/signal.Float64.Append
/Users/dudk/gocode/pkg/mod/pipelined.dev/[email protected]/float64.go

  Total:      1.59GB     1.61GB (flat, cum) 99.46%
    103            .          .           // new buffer will be allocated with capacity of both sources. 
    104            .          .           func (s Float64) Append(src Floating) Floating { 
    105            .          .           	mustSameChannels(s.Channels(), src.Channels()) 
    106            .          .           	if s.Cap() < s.Len()+src.Len() { 
    107            .          .           		// allocate and append buffer with cap of both sources capacity; 
    108       1.59GB     1.59GB           		s.buffer = append(make([]float64, 0, s.Cap()+src.Cap()), s.buffer...) 
    109            .          .           	} 
    110            .          .           	result := Floating(s) 
    111            .          .           	for i := 0; i < src.Len(); i++ { 
    112            .       21MB           		result = result.AppendSample(src.Sample(i)) 
    113            .          .           	} 
    114            .          .           	return result 
    115            .          .           } 
    116            .          .            
    117            .          .           // Slice slices buffer with respect to channels. 

The mentioned code explicitly creates new slice and copies over content of the source slice. It's done to have full control on result slice capacity. There are multiple ways to address this problem, going to list them in the follow ups.

from audio.

So this issue actually is very similar to a problem in the current stable version of the AWS SDK for Go. They used a similar strategy for allocating buffer space when downloading files and it caused their SDK to use huge amounts of memory. There is a fix I think in the beta version for it. They also supported a buffer pool solution that reused memory pools to avoid re-allocating on the heap.

A few related issues:
aws/aws-sdk-go#3085
aws/aws-sdk-go#3035

from audio.

To me it looks like because of the use of the pipe and lines to copy a wav to an asset there is a load of allocs that don't need to happen there. Feels like the wav source could be used directly reducing the multiple allocs for the just loading the wav file before mixing.

from audio.

Okay, so the problem is clear: new buffer is allocated on every audio.Asset.Sink call.

Here are the exact steps that lead to this behaviour:

1. signal.Floating interface is used to hold audio.Asset data during sinking;
2. on every sink call audio.Asset calls signal.Floating.Append;
3. signal.Floating.Append checks if buffer has enough capacity and a) uses existing slice to put data if true, b) allocates new slice with sum of sources capacities and copies data over there;
4. because audio.Asset doesn't have any pre-allocation mechanisms, it causes allocation of new buffer on every sink call.

Possible fixes:

Option 1: use []float64 instead of signal.Floating to store data within asset during sinking.

It'll prevent extensive allocations because new buffer won't be allocated on every sink call. On top of that it will enable default mechanics of slices growth in go.

Option 2: introduce pre-allocation parameter (buffer) for asset.Sink.

Actually, for this option everything is in place already - preallocated asset.Floating buffer can be used. asset.Sink just need to handle preallocated buffer if any present.

Option 3: change signal.Floating.Append function to not copy slice every time, but use built-in append underneath.

This will solve problem of extensive allocation and will allow to handle situation when size of the buffer isn't known upfront.

The challenge with option 1 is that other APIs are relying on pipelined.dev/signal types to manipulate signal data, including audio.Track. It means that after sinking is done, []float64 buffer should be wrapped into signal.Floating type. I'll create new API for this in signal package.

I would go to option 3 as it will improve performance for most situations, where signal.Floating.Append might be used. Need to think a bit about possible implementation for it.

from audio.

I tested out a very quick change to asset to use a float64 slice and it does drop the memory allocation by nearly half. But it's still too high. I think there is also an allocation between the source (wav in my case) to the asset via pipe.

This might be unavoidable to keep the API sane but it looks at a glance as though we allocate to a signal.Floating in the wav source functions which are then copied to an allocator with a sized buffer. This is then copied into the asset via another float64 buffer ([]float64 in my testing case) so it's allocating the memory needed for the PCM data multiple times still.

I'd propose that alongside the changes to how signal.Floating works to avoid extra allocation maybe a look at how assets are populated in the first place, maybe they can use the Source() function from wav / mp3 / flac directly rather than copying via pipe into the asset, which feels a little clunky and unclear at the moment anyway

from audio.

Or is asset even required? since as I mentioned before I think the wav / flac / mp3 packages could implement the asset interface and so be directly piped into lines without needing the extra steps / copies to get the source into the asset.

Personally feels like a cleaner and more logical API.

from audio.

I think that we have a bit of confusion here :) The purpose of asset is specifically to put signal data into memory, so the user can manipulate it - slice it, compose tracks with it. If you want to mix wav/mp3 data as it is, you don't need to use asset at all.

Let's go back to your particular example.

1. Read wav file into asset - once problem of signal.Floating.Append is fixed, number of allocations will go down significantly and only single copy of original file will be kept in memory.
2. Put references of the asset into tracks - only signal.Floating instances will be allocated, no data copied.
3. Mix the track signals into new wav file.

Because you use the same wav file, it doesn't make sense to read it multiple times from the disk - it's much easier to read it into asset once before manipulating with it. If you are going to have a 100 of wav files and you want to just mix them all together - you can use it with mixer and ignore asset completely.

Let me know if it makes sense.

from audio.

Perhaps we do. So my examples are very contrived but my real use case is this.

A mix down is created from multiple tracks of multiple wav files.

So like a traditional multitrack mixer. Each wav file would need to be loaded into memory fair. However with the current api I see no way to skip using asset as the wav source doesn’t current provide a way for the track to interact with it that I can see. The onto api I could see that does what I need is using AddClip with signal.Floating which requires routing from wav into asset first.

I have a fork that I have an quick and dirty experiment to remove the extra allocations in sink but there is still and extra copy between wav -> asset -> line.

from audio.

A mix down is created from multiple tracks of multiple wav files.

I think that this is where my understanding was diverged. I have more of an sequencer track concept in mind during development of it. Current audio.Track implementation suggests that you are dealing with precise pieces of signal. It means that every time a clip is added - it has certain duration and position in the track.

And for your use case it seems like you don't really care about picking a segment of the file or taking into account it's duration, right? You just want to have a sequence of wav files per mixer track, is that right?

from audio.

In a simple use case yes. But being able to take a segment of a file / take into account it's duration is valuable. Especially with being able to pass it to processors

from audio.

It looks like an interesting use case. I see multiple problems that you might face here:

1. Not all audio formats provide duration of the signal, mp3 is a good example. This means that for "generic" solution you will have to consider to read whole file. It's not really necessary to keep it all in memory though.
2. Currently, there is no "limited" source that will read only up to certain duration of the file. I think it's fairly easy to write such a generic source wrapper though.

Maybe you can share more context about processors use case?

from audio.

I can, but because what we're working on isn't public domain at the moment i'm hesitant to share details on an open forum such as GitHub issues.

I'll send you an email.

from audio.

So, I did couple of tests. I used your sample project, @haswalt but I adjusted it to load just a single asset and use just a single track with a single clip.

First, I changed signal.Float64.Append function to look like this:

func (s Float64) Append(src Floating) Floating {
	mustSameChannels(s.Channels(), src.Channels())
	offset := s.Len()
	if s.Cap() < s.Len()+src.Len() {
		// allocate and append buffer with cap of both sources capacity;
		s.buffer = append(s.buffer, make([]float64, src.Len())...)
	} else {
		s.buffer = s.buffer[:s.Len()+src.Len()]
	}
	for i := 0; i < src.Len(); i++ {
		s.SetSample(i+offset, src.Sample(i))
	}
	alignCapacity(&s.buffer, s.Channels(), s.Cap())
	return s
}

// alignCapacity ensures that buffer capacity is aligned with number of channels.
func alignCapacity(s interface{}, channels, cap int) {
	if r := cap % channels; r != 0 {
		reflect.ValueOf(s).Elem().SetCap(cap - r)
	}
}

Now memory profile looks like this:

Total:     20.96MB    20.96MB (flat, cum) 77.74%
    104            .          .           func (s Float64) Append(src Floating) Floating { 
    105            .          .           	mustSameChannels(s.Channels(), src.Channels()) 
    106            .          .           	offset := s.Len() 
    107            .          .           	if s.Cap() < s.Len()+src.Len() { 
    108            .          .           		// allocate and append buffer with cap of both sources capacity; 
    109      20.96MB    20.96MB           		s.buffer = append(s.buffer, make([]float64, src.Len())...) 
    110            .          .           	} else { 
    111            .          .           		s.buffer = s.buffer[:s.Len()+src.Len()] 
    112            .          .           	} 
    113            .          .           	for i := 0; i < src.Len(); i++ { 
    114            .          .           		s.SetSample(i+offset, src.Sample(i)) 

It's a significant drop from 1.6GB to load just a single asset to the memory. However, I was still a bit concerned that the file size is 1.3MB and it takes 21MB to load it to the memory.

Then I did another change to enable preallocation for the asset:

asset.go

// Sink appends buffers to asset.
func (a *Asset) Sink() pipe.SinkAllocatorFunc {
	return func(bufferSize int, props pipe.SignalProperties) (pipe.Sink, error) {
		a.SampleRate = props.SampleRate
		if a.Floating == nil {
			a.Floating = signal.Allocator{
				Channels: props.Channels,
				Capacity: bufferSize,
			}.Float64()
		}
		return pipe.Sink{
			SinkFunc: func(in signal.Floating) error {
				a.Floating = a.Floating.Append(in)
				return nil
			},
		}, nil
	}
}

main.go

asset := audio.Asset{
    Floating: signal.Allocator{
        Capacity: 330534,
        Channels: 2,
    }.Float64(),
}

The profile:

  Total:      5.05MB     5.05MB (flat, cum) 34.67%
     40            .          .           			asset := audio.Asset{ 
     41            .          .           				Floating: signal.Allocator{ 
     42            .          .           					Capacity: 330534, 
     43            .          .           					Channels: 2, 
     44            .          .           				}.Float64(), 
     45       5.05MB     5.05MB           			} 
     46            .          .            
     47            .          .           			// This feels like an unnessecary extra step and causes a copy 
     48            .          .           			l, _ := pipe.Routing{ 
     49            .          .           				Source: source.Source(), 
     50            .          .           				Sink:   asset.Sink(), 

Now no additional allocations happen in the asset during pipe execution. The difference between in asset (5MB) and wav file (1.3MB) happens because asset uses float64 to store signal and example wav file is int16. I'll proceed with fixes for signal and audio packages.

pipelined.dev/signal package offers functions for any-to-any formats (signed/unsigned fixed and floating). It should be fairly simple to add different types of the assets, so the most suitable can be used to reduce memory footprint at runtime.

from audio.

@dudk awesome! that's a huge improvement!

from audio.

yes, that Append implementation was absolutely broken.

from audio.

Hello @haswalt, the release is done for all pipelined components. I'm closing it for now, feel free to open new issue with questions/feedback. Thanks!

from audio.