Move the run-length encoding under a feature

[Kerollmops]

I propose introducing a new feature to disable run-length encoding in certain cases. It is enabled by default. I am wondering about the naming now, as I find run-length-serialization a bit long and ugly.

[lucascool12]

Wouldn't it be better to add a new method that serializes without run-length encodings? Using features to enable or disable makes it impossible to choose this at runtime, which I see no real reason to not allow.

[Kerollmops]

Hey @lucascool12 👋

You are right, I think it would be better to have a serialization_with_options method. What do you think? The options would be a struct with different fields, and the first one would be a simple run_length_encoding boolean.

Thanks for the help and have a nice day 🌵

[lucascool12]

You are right, I think it would be better to have a serialization_with_options method. What do you think? The options would be a struct with different fields, and the first one would be a simple run_length_encoding boolean.

That seems like the best approach. I'm not sure what the best way to make this struct backward compatible is. I first thought a normal struct with #[non_exhaustive] would be a good idea, but after some light reading of https://doc.rust-lang.org/reference/attributes/type_system.html#r-attributes.type-system.non_exhaustive I no longer think this is a good idea. Maybe some builder like thing would work.

[Dr-Emann]

Is there a reason calling remove_run_compression() before serializing isn't acceptable?

[Kerollmops]

Hey @Dr-Emann 👋

Sorry for the very late answer. I was taking holidays.

Is there a reason calling remove_run_compression() before serializing isn't acceptable?

Yes, the main reason is because our serialization in Meilisearch receives an immutable reference of a roaring bitmap (&RoaringBitmap). We can't modify the bitmap in place and cloning it would really have a huge perf impact on the indexing phase.

---

Hey @lucascool12 👋

[about a non_exhaustive struct] I no longer think this is a good idea. Maybe some builder like thing would work.

I think you're right and the approach would probably to introduce a new builder pattern like the std OpenOptions one is a better idea 🤔 But I definitely think using a feature is not the right solution, anyway.