You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "scovich (via GitHub)" <gi...@apache.org> on 2025/06/30 19:56:09 UTC
[I] [Variant] VariantMetadata, VariantList and VariantObject are too big for Copy [arrow-rs]
scovich opened a new issue, #7831:
URL: https://github.com/apache/arrow-rs/issues/7831
**Is your feature request related to a problem or challenge? Please describe what you are trying to do.**
`VariantMetadata` is currently 40 bytes large. It's questionable whether that's _really_ an appropriate size for a `Copy` type.
Worse, `VariantList` and `VariantObject` (also `Copy`) both take `VariantMetadata` by value. This makes lifetimes easier to manage, but bloats them up to and 88 and 96 bytes, respectively.
**Describe the solution you'd like**
We should consider storing fewer fields (`first_offset_byte` is very cheap and safe to compute from other header information), and also storing `u32` instead of `usize` -- variant only uses 32-bit indexing, so we shouldn't double the physical size just for convenience of using `usize` on 64-bit architectures.
Doing that would give the following size reductions:
* `VariantMetadata`: 40 => 32 bytes
* `VariantList`: 88 => 64 bytes
* `VariantObject`: 96 => 64 bytes
NOTE: Since we're storing byte slices, we're stuck with 8-byte alignment and it would be ~impossible to go smaller. It's still debatable whether these are really `Copy` types, but at least they would fit in a typical CPU cache line now?
**Describe alternatives you've considered**
We could reduce the footprint of object and list by storing `VariantMetadata` by reference instead of by value. But if we shrink `VariantMetadata` that would only save 16 bytes while complicating the user surface with additional lifetime management worries.
We may even want to consider using u32 offsets everywhere, since that's actually what the variant spec mandates. That would also make the arithmetic overflow issue easier to reason about and test, because 32- and 64-bit architectures would both behave the same way. But rust really likes `usize` for indexing operations, so this may also complicate the user surface.
**Additional context**
N/A
--
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.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] [Variant] VariantMetadata, VariantList and VariantObject are too big for Copy [arrow-rs]
Posted by "scovich (via GitHub)" <gi...@apache.org>.
scovich commented on issue #7831:
URL: https://github.com/apache/arrow-rs/issues/7831#issuecomment-3020540274
> We could reduce the footprint of object and list by storing `VariantMetadata` by reference instead of by value. But if we shrink `VariantMetadata` that would only save 16 bytes while complicating the user surface with additional lifetime management worries.
Actually,`&VariantMetadata` is a thin pointer, so storing a reference would save 24 bytes (32 => 8 bytes). Combining that with the previously suggested changes would reduce `VariantList` and `VariantObject` to 40 bytes each.
--
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
Re: [I] [Variant] VariantMetadata, VariantList and VariantObject are too big for Copy [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #7831:
URL: https://github.com/apache/arrow-rs/issues/7831#issuecomment-3020854762
How about we just make them `Clone` rather than `Copy`?
I don't think it is possible to maek `VariantMetadata` a reference without making it quite a bit harder to construct Variants (b/c you can't make them directly from the metadata/value you first need to make a `VariantMetadata`
--
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
Re: [I] [Variant] VariantMetadata, VariantList and VariantObject are too big for Copy [arrow-rs]
Posted by "scovich (via GitHub)" <gi...@apache.org>.
scovich commented on issue #7831:
URL: https://github.com/apache/arrow-rs/issues/7831#issuecomment-3029519432
> How about we just make them `Clone` rather than `Copy`?
>
> I don't think it is possible to maek `VariantMetadata` a reference without making it quite a bit harder to construct Variants (b/c you can't make them directly from the metadata/value you first need to make a `VariantMetadata`
Yeah, I think the sweet spot will be to slim them down to use fewer bytes but not try to pass a reference. If we revoke `Copy` and only leave them `Clone`, the whole `Variant` will also need the same treatment. Is that acceptable?
--
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
Re: [I] [Variant] VariantMetadata, VariantList and VariantObject are too big for Copy [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #7831:
URL: https://github.com/apache/arrow-rs/issues/7831#issuecomment-3130011971
`label_issue.py` automatically added labels {'parquet'} from #7878
--
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
Re: [I] [Variant] VariantMetadata, VariantList and VariantObject are too big for Copy [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #7831: [Variant] VariantMetadata, VariantList and VariantObject are too big for Copy
URL: https://github.com/apache/arrow-rs/issues/7831
--
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
Re: [I] [Variant] VariantMetadata, VariantList and VariantObject are too big for Copy [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #7831:
URL: https://github.com/apache/arrow-rs/issues/7831#issuecomment-3032509246
> > How about we just make them `Clone` rather than `Copy`?
> > I don't think it is possible to maek `VariantMetadata` a reference without making it quite a bit harder to construct Variants (b/c you can't make them directly from the metadata/value you first need to make a `VariantMetadata`
>
> Yeah, I think the sweet spot will be to slim them down to use fewer bytes but not try to pass a reference. If we revoke `Copy` and only leave them `Clone`, the whole `Variant` enum will also need the same treatment. Is that acceptable?
I think removing Copy from Variant is acceptable -- it will make people more aware about cloning
I agree that slimming the objects down *AND* removing `Copy` sounds good
--
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