Eventual-Inc / Daft

Distributed DataFrame for Python designed for the cloud, powered by Rust
https://getdaft.io
Apache License 2.0
2.09k stars 141 forks source link

Field name use `Arc<str>` instead of `String` #2892

Open andrewgazelka opened 5 hours ago

andrewgazelka commented 5 hours ago

https://github.com/Eventual-Inc/Daft/blob/08ca9a4078e4506afc9b774bd2f073eee94a38d9/src/daft-schema/src/field.rs#L17

raunakab commented 5 hours ago

I would support converting over to Arc<str> as well. I believe there was a cool YouTube video referencing why Arc<str>s are more preferable in many situations than just regular Strings.

The most applicable point here is that I believe the actual field names are never in-place mutated. So yes, would make more sense to Arc<str> it.

raunakab commented 5 hours ago

Found it: https://www.youtube.com/watch?v=A4cKi7PTJSs.

samster25 commented 4 hours ago

@andrewgazelka I think this makes sense since we often clone Fields which could cause a redundant memory allocation for String