-
-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Support homedir expansion in lazy/scan read functions #16869
base: main
Are you sure you want to change the base?
feat: Support homedir expansion in lazy/scan read functions #16869
Conversation
CodSpeed Performance ReportMerging #16869 will not alter performanceComparing Summary
|
.iter() | ||
.map(|p| resolve_homedir(p)) | ||
.collect::<Vec<_>>() | ||
.into(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not entirely sure, but I believe we can collect into a Arc<[]>
directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(thinking out loud)
Will poke at it some more 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, can't see a way to avoid the intermediate Vec
; the Arc
seems to want a constructed object to reference? 🤔 If you can spot something cunning, feel free to point me at it or commit over the top :))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now I am sure. :D
Lol... pretty sure that the collect
there still creates an intermediate Vec
, it just gets deallocated after conversion to Arc[i32]
:)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but Rust might optimize that later.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16869 +/- ##
==========================================
+ Coverage 81.36% 81.42% +0.06%
==========================================
Files 1425 1425
Lines 187910 187988 +78
Branches 2705 2704 -1
==========================================
+ Hits 152885 153070 +185
+ Misses 34529 34421 -108
- Partials 496 497 +1 ☔ View full report in Codecov by Sentry. |
3342ad1
to
65973e7
Compare
65973e7
to
d236a44
Compare
@alexander-beedie Just wondering if Bottom line: if the user calls |
(@ritchie46: as mentioned yesterday)
We already expand the homedir
~/
for eager read function paths, but seem to have missed it for the lazy/scan equivalents; this adds the missing expansion there too.