search

cberner / raptorq

Rust implementation of RaptorQ (RFC6330)
Apache License 2.0
285 stars 47 forks source link

Sikabo master #37

Closed Sikabo closed 4 years ago

Sikabo commented 4 years ago

Hi,

Two commits. The SSSE3 is straight forward and nothing special. I don't have AVX2 on my Linux machine at home so I always run SSSE3.

You have more or less already added an operation vector so I reworked my code to clone your operation vector and add a "reorder" operation. My Rust isn't the best so feel free to rewrite and restructure the code.

Great work with the recent sparse matrix improvements.

cberner commented 4 years ago

Thanks! I merged your SSSE3 commit. I need to read through the operation vector commit in more detail. It mostly looks good, but I'm not sure about having the user manage the SourceBlockEncoderCache

Sikabo commented 4 years ago

My usage scenario is probably different. I have several SourceBlockEncoder running in different threads and they could all be working with different block-sizes. I share the cache between all of them.

cberner commented 4 years ago

Your use case makes a lot of sense, and improving support for it sounds good. I'd like to avoid adding cache management into the raptorq crate though, as I think that may get quite complex and the best implementation depends on the needs of the workload.

Instead of adding SourceBlockEncoderCache that's shared, thread-safe, and stores all sizes, I think we should add a SourceBlockEncodingPlan which holds the Vec<SymbolOps> and any other information it needs. That can be calculated via a separate pub function, and then you can store and cache it in your implementation. The plan can then be passed into SourceBlockEncoder::new as an Option and used as the operation vector.

What do you think of that?

Sikabo commented 4 years ago

Sounds good. This would make the cache handling much more flexible.

cberner commented 4 years ago

Great. Not sure if I'll have time to work on that this weekend, but I'll make those changes and merge your PR soon.

cberner commented 4 years ago

Merged. Thanks! Let me know if you have any issues with the new API

Sikabo commented 4 years ago

Nice, I will let you know how it works for me. I think it will suit my needs.