Is your feature request related to a problem or challenge?
While updating DataFusion to arrow 60 I found the new Metadata struct introduced in #10075 does not expose its underlying reference-counted map. This hurts in two places:
- Memory accounting: DataFusion tracks heap usage of plans/schemas and dedupes allocations shared via
Arc by pointer. Since Metadata hides its allocation, DataFusion can only approximate its size and double-counts clones that share the same map:
impl DFHeapSize for Metadata {
fn heap_size(&self, ctx: &mut DFHeapSizeCtx) -> usize {
// `Metadata` does not expose its underlying reference-counted map, so
// this approximates the `BTreeMap` entries' sizes and cannot dedupe
// instances that share the same allocation.
self.iter()
.map(|(k, v)| {
size_of::<(String, String)>() + k.heap_size(ctx) + v.heap_size(ctx)
})
.sum()
}
}
- Zero-copy conversion to wrapper types: DataFusion's
FieldMetadata also stores an Arc<BTreeMap<String, String>>. The inbound direction is cheap (Metadata: From<Arc<BTreeMap<String, String>>>), but the outbound direction has to go through BTreeMap, which clones the map whenever the Arc is shared:
impl From<Metadata> for FieldMetadata {
fn from(value: Metadata) -> Self {
// From<Metadata> for BTreeMap clones the map when the Arc is shared
Self::new(value.into())
}
}
Describe the solution you'd like
Accessors that expose the shared map, for example Metadata::as_arc(&self) and Metadata::into_arc(self). Then the conversion is always cheap:
impl From<Metadata> for FieldMetadata {
fn from(value: Metadata) -> Self {
Self::new_from_arc(value.into_arc())
}
}
and memory accounting can dedupe by allocation:
impl DFHeapSize for Metadata {
fn heap_size(&self, ctx: &mut DFHeapSizeCtx) -> usize {
let map = self.as_arc();
if !ctx.count_allocation_once(Arc::as_ptr(map)) {
return 0; // already counted a clone sharing this allocation
}
map.iter().map(|(k, v)| ...).sum()
}
}
Describe alternatives you've considered
Approximating the size and accepting the extra clone, as shown above.
Additional context
Is your feature request related to a problem or challenge?
Metadatastruct instead ofHashMap<String, String>for metadata: Ordered; cheap to clone #10075While updating DataFusion to arrow 60 I found the new
Metadatastruct introduced in #10075 does not expose its underlying reference-counted map. This hurts in two places:Arcby pointer. SinceMetadatahides its allocation, DataFusion can only approximate its size and double-counts clones that share the same map:FieldMetadataalso stores anArc<BTreeMap<String, String>>. The inbound direction is cheap (Metadata: From<Arc<BTreeMap<String, String>>>), but the outbound direction has to go throughBTreeMap, which clones the map whenever theArcis shared:Describe the solution you'd like
Accessors that expose the shared map, for example
Metadata::as_arc(&self)andMetadata::into_arc(self). Then the conversion is always cheap:and memory accounting can dedupe by allocation:
Describe alternatives you've considered
Approximating the size and accepting the extra clone, as shown above.
Additional context