You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/12/01 21:17:56 UTC
[GitHub] [arrow-rs] carols10cents opened a new issue #989: serde `rc` feature might not be needed?
carols10cents opened a new issue #989:
URL: https://github.com/apache/arrow-rs/issues/989
This is low priority, and I'm happy to send in a PR for this if it's determined we can remove the `rc` feature.
I'm analyzing dependencies and features in IOx, which is also leading me to analyze dependencies and features in IOx's dependencies.
I noticed arrow enables serde's `rc` feature: https://github.com/apache/arrow-rs/blob/e9be49d962560ce5b87544a2933d8b207322cf60/arrow/Cargo.toml#L40
[serde's docs say](https://serde.rs/feature-flags.html#-features-rc):
> --features rc
>
> Opt into impls for Rc<T> and Arc<T>. Serializing and deserializing these types does not preserve identity and may result in multiple copies of the same data. Be sure that this is what you want before enabling this feature.
>
> Serializing a data structure containing reference-counted pointers will serialize a copy of the inner value of the pointer each time a pointer is referenced within the data structure. Serialization will not attempt to deduplicate these repeated data.
>
> Deserializing a data structure containing reference-counted pointers will not attempt to deduplicate references to the same data. Every deserialized pointer will end up with a strong count of 1.
The feature was added in [this PR](https://github.com/apache/arrow/pull/3016) made 3 years ago. It adds `derive(Serialize, Deserialize)` to `DataType`, `Field`, and `Schema`, none of which contain any `Rc<T>` or `Arc<T>` as far as I can tell. It only adds a test for a `DataType::Struct` that contains `Field`s, but `Schema` contains `Field`s too, so I think that coverage is enough.
I tried taking the `rc` feature out and arrow's tests still pass.
@andygrove I don't suppose you remember why you enabled the `rc` feature of `serde` in a PR made 3 years ago? 😂
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] alamb closed issue #989: serde `rc` feature might not be needed?
Posted by GitBox <gi...@apache.org>.
alamb closed issue #989:
URL: https://github.com/apache/arrow-rs/issues/989
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] houqp commented on issue #989: serde `rc` feature might not be needed?
Posted by GitBox <gi...@apache.org>.
houqp commented on issue #989:
URL: https://github.com/apache/arrow-rs/issues/989#issuecomment-984226973
if it builds without `rc` and passes all tests, then I don't see a reason to keep it :D
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org