Skip to content

Code Quality: Improved nav perf - #18934

Open
yair100 wants to merge 3 commits into
mainfrom
ya/row-render-perf
Open

Code Quality: Improved nav perf#18934
yair100 wants to merge 3 commits into
mainfrom
ya/row-render-perf

Conversation

@yair100

@yair100 yair100 commented Sep 8, 2026

Copy link
Copy Markdown
Member

Resolved / Related Issues

To prevent extra work, all changes to the Files codebase must link to an approved issue marked as Ready to build. Please insert the issue number following the hashtag with the issue number that this Pull Request resolves.

  • Closes #

Steps used to test these changes

Stability is a top priority for Files and all changes are required to go through testing before being merged into the repo. Please include a list of steps that you used to test this PR.

  1. Opened Files ...
  2. ...

@yair100
yair100 requested a review from hez2010 September 8, 2026 21:40
@yair100 yair100 added the ready for review Pull requests that are ready for review label Sep 8, 2026

// Speculative directory open started at navigation request time so it overlaps the frame-navigation window.
private sealed record DirectoryOpenPrefetch(string Path, Task<(Win32PInvoke.SafeFindHandle? Handle, WIN32_FIND_DATA Data, int ErrorCode)> Task);
private static DirectoryOpenPrefetch? directoryOpenPrefetch;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The prefetch might seem like a small gain, but it's better than nothing. I would be happy to skip this change in favor of a better idea.

@hez2010 hez2010 Sep 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather like to remove this prefetch. Implementing a cache also needs proper cache invalidations, which would then complicate the whole thing by a lot.

For example, with this change refreshing a folder can reuse an old scan, which can then lead to missing files. And a drive letter that was previously used by a local drive could later be remapped to a network share while Files is still open. They both lead to correctness regressions.

IMO the risk here is larger than the perf gain.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an alternative solution? There is often a short gap between double clicking a folder and the items rendering. It's not a long gap but enough to be noticed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to profile this path first to identify what is blocking the navigation from happening.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review Pull requests that are ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants