-
Notifications
You must be signed in to change notification settings - Fork 26
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
Comments
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 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. |
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. |
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 I have profiled the 'cmp bench' command and it seems that |
Hello @fdoyon and thanks for your interest in lewton.
Backwards compatibility isn't even the reason for the usage of
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 |
Great, thanks, I will start hacking on my end with your guidelines.
… On 24 Jan 2019, at 14:35, est31 ***@***.***> wrote:
Hello @fdoyon <https://github.com/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 <https://github.com/RustAudio/lewton/blob/8143702864baeb24dd57d889581fceb35ad33c40/src/lib.rs#L33-L40> 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 <#12>. I suppose we would use either the FromSample <https://docs.rs/sample/0.10.0/sample/conv/trait.FromSample.html> trait or something else. The flac crate <https://docs.rs/flac/0.5.0/flac/trait.Sample.html> 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 <RustAudio/dasp#108> PR is accepted, I think that lewton will be able to use the sample crate. But let's keep further discussion in #12 <#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.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#40 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACBisVsN1SG5kstZTj0ymCNpwbqtkPjks5vGcS-gaJpZM4ZU-lM>.
|
@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. |
@fdoyon I've pushed some commits (8143702...a837856) to make lewton generic in the output format. |
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.
The text was updated successfully, but these errors were encountered: