BurntSushi / walkdir

Rust library for walking directories recursively.
The Unlicense
1.22k stars 107 forks source link

Implement From conversions for WalkDir #94

Closed nvzqz closed 5 years ago

nvzqz commented 6 years ago

Allows for creating a WalkDir from an existing PathBuf without allocating and copying from a Path slice.

Also added From<&Path> for consistency with WalkDir::new.

BurntSushi commented 6 years ago

These impls seem a touch weird to me, but I don't feel strongly. What is the motivating use case for these?

nvzqz commented 6 years ago

My main motivation is that a new PathBuf gets allocated if one gets passed into WalkDir::new. This allows for using an existing allocation.

BurntSushi commented 6 years ago

Do you have a real benchmark that shows this one particular allocation matters? The rest of the implementation is careful with allocation, but there is definitely plenty more of it. I have a hard time believing that the removal of one allocation matters here, particularly when there are many other allocations and syscalls.

nvzqz commented 6 years ago

No I do not. It just stood out to me as odd that there wasn't an efficient way to pass a buffer without creating a new one.

I don't think these are out of place, however. It's common for types to have From implementations that are symmetrical to what can be passed into new.

BurntSushi commented 5 years ago

I think I'd rather hold off on taking these impls for now until we can come up with more compelling motivation for them. (e.g., A benchmark or specific use case.)