Repository navigation
Avoid extra SMB queries when formatting file names - #28038
Dongbo Wang (daxian-dbw) merged 1 commit into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
No unresolved review issues were identified, and regression coverage was added.
Review effort: Lite
Findings: None
What changed in this PR
Improves file-name formatting performance by avoiding unnecessary SMB metadata queries for ordinary items.
Changes:
- Uses cached attributes to short-circuit reparse-point inspection.
- Adds regression coverage for cached metadata behavior.
| File | Description |
|---|---|
test/powershell/Modules/Microsoft.PowerShell.Management/Get-Item.Tests.ps1 |
Verifies cached non-reparse items do not reopen their paths. |
src/System.Management.Automation/namespaces/FileSystemProvider.cs |
Avoids unnecessary metadata queries during formatting. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
Justin Chung (jshigetomi)
left a comment
There was a problem hiding this comment.
Reviewed the cached-attribute guard, reparse-point handling, and regression coverage. Looks good to me.
| // Checking the cached attributes first avoids an extra filesystem query for every non-link, | ||
| // which is particularly expensive when formatting items on an SMB share. | ||
| if (InternalSymbolicLinkLinkCodeMethods.IsReparsePoint(fileInfo) | ||
| && InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(fileInfo)) |
There was a problem hiding this comment.
The IsReparsePointLikeSymlink method is used in RemoveDirectoryInfoItem(), so I guess it makes sense to insert this check there too.
There was a problem hiding this comment.
That's out of scope for this PR. This PR is only for fixing the formatting.
c4ae347
into
PowerShell:master
PR Summary
NameString.FindFirstFilequery per ordinary item when formatting directory listings, which is particularly expensive on SMB shares.PR Context
Fixes #27088.
IsReparsePointLikeSymlinkmust inspect the reparse tag to distinguish symbolic links and junctions from cloud-drive placeholders. Default formatting previously invoked that expensive check for every item, including ordinary non-links.NameStringnow uses the already-populatedAttributesmetadata to short-circuit ordinary items while preserving detailed classification for actual reparse points.Benchmark
Measured 4,897 entries over the local UNC path
\\localhost\c$\Windows\System32. Each result is the median of three warmed-up runs.NameStringA same-binary control using the fixed debug build isolates the avoided operation:
NameStringIsReparsePointLikeSymlinkprobeThis confirms that the approximately 3.9-second cost came from calling
FindFirstFileonce per item. Absolute formatting times vary between release and debug builds, but the per-item SMB query is eliminated.PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright header