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

Make lewton avoid allocations #40

Open
est31 opened this issue Dec 16, 2018 · 7 comments
Open

Make lewton avoid allocations #40

est31 opened this issue Dec 16, 2018 · 7 comments

Comments

@est31
Copy link
Member

est31 commented Dec 16, 2018

In general, allocations are an overhead. lewton right now uses the allocator quite heavily.
It would be great to reduce the amount of allocations that lewton is doing.
This might also help with other things like this issue: #34

Recently, @raphlinus has given an interesting talk about a bunch of things, including low-latency audio programming. He mentioned the idea that in order to check that code indeed avoids allocations, one could create a global allocator that panics/aborts if there is an allocation happening. Something like this (using the global allocator) could be used by lewton as well to ensure that there are no/few allocations. There won't be a static guarantee, but there'll be a dynamic one.

@est31 est31 changed the title Make lewton avoid more allocations Make lewton avoid allocations Dec 16, 2018
@Shnatsel
Copy link
Contributor

Shnatsel commented Dec 16, 2018

AFAIK libraries cannot have their own allocators - there should be just one global allocator in the entire binary (which is why it's called "global"). So you could use this technique in test binaries, but not in actual programs at runtime.

Using Rust without the standard library (aka in [#no_std] mode) would also avoid allocations, as your code would lack an allocator altogether. That way you have a static assurance. But that way you'll also be missing some stdlib types, and the allocations you always want to make (i.e. static, not dynamic ones) might be too large for the stack, so you'd have to put them in a boxed slice, which would require an allocator.

In theory it should be possible to write a Vorbis decoder using iterators only, but the quality of the generated code was pretty bad last time I checked (admittedly, a while ago), so you might want to preallocate a fixed amount of scratch space. Although I'm pretty sure there was some kind of low-level library that used iterators only, without allocations, and that made it faster than the C equivalent, but I don't remember what it was.

@est31
Copy link
Member Author

est31 commented Dec 17, 2018

Yeah, the allocator trick would only be used for test binaries that will be fed the set of test files that lewton uses.

Also, for now my goal is to avoid allocations during the actual audio decoding. Header decoding is a different question: here I think that avoiding allocations has the highest price.

@fdoyon
Copy link

fdoyon commented Jan 24, 2019

I have started a fork which removes some of the allocations but I am fighting against the design and would need to change some of the design choices of the public API.

May I ask why the code still uses the try! macros (backwards-compatibility?) and what the stance of the maintainers is relative to adding a dependency on the Sample crate.

There's a lot of work being done by the codec to convert output formats (int16/f32, interleaved vs multiple channels) that could be abstracted away and handled in a more efficient manner for the end-user by relying on the Sample abstractions in the public API.

For example my app needs to get the interleaved samples out of an ogg/vorbis stream in f32 stereo interleaved form, so there's a wasteful f32->i16 conversion done in Lewton.

I have profiled the 'cmp bench' command and it seems that lewton is on par with libvorbis and that most of the extra time is spent allocating / dropping Vec and SmallVec instances.

@est31
Copy link
Member Author

est31 commented Jan 24, 2019

Hello @fdoyon and thanks for your interest in lewton.

May I ask why the code still uses the try! macros

Backwards compatibility isn't even the reason for the usage of try!. The minimum supported Rust version of lewton is 1.20.0 and it already has ?. The historic reason is that I've only recently warmed up to the ? operator, reluctantly, before that I was avoiding it. But the big reason for continuing to use try! is because try is giving me a cheap way to track down errors, i.e. without using the backtrace crate or something similar. I'm using this macro and put it into the piece of code that is interesting to me. You can't do something similar for the ? operator. There is simply no added value in switching lewton to ? or whatever.

stance of the maintainers is relative to adding a dependency on the Sample crate.

The sample type issue is tracked on github in #12. I suppose we would use either the FromSample trait or something else. The flac crate has made its own sample trait. Generally I'm trying to keep the number of dependencies low to allow fast compilation of lewton. So if sample were a fat crate with tons of dependencies and such I'd say that we should roll our own traits, but sample has zero dependencies. So I would say that I'm undecided. The only reason that comes to my mind to avoid the sample crate is if they have an MSRV policy incompatible to our own. If this PR is accepted, I think that lewton will be able to use the sample crate. But let's keep further discussion in #12.

As for the public API, it's not stable. It can be changed whenever we want to change it. Especially if it is bringing us performance improvements! I suggest you file a PR that changes the public API and we can discuss the details.

For coding style, note that lewton uses tabs for indentation and item :Type notation and Struct { member : value } one.

@fdoyon
Copy link

fdoyon commented Jan 24, 2019 via email

@est31
Copy link
Member Author

est31 commented Feb 5, 2019

@fdoyon don't feel blocked by the sample MSRV discussion: just file a PR that uses sample. In the worst case, we'll remove the sample dependency and roll our own traits.

@est31
Copy link
Member Author

est31 commented Feb 21, 2019

@fdoyon I've pushed some commits (8143702...a837856) to make lewton generic in the output format.

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

No branches or pull requests

3 participants