Skip to content
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

candidate fix for slowness on NFS #1758

Merged
merged 2 commits into from Nov 16, 2017
Merged

Conversation

kevinushey
Copy link
Contributor

@kevinushey kevinushey commented Nov 16, 2017

This is a candidate fix for the slowness observed by some Windows users running on networked filesystems, re: #1592.

In v1.1, we introduced a feature that attempts to respect a parent project configuration when opening a file located outside of the current project. We do this by looking up the filesystem for files with the extension .Rproj, which implies recursively listing the contents of directories until a directory is found.

Unfortunately, there were two problems:

  1. Newly-created documents don't have a path; calling resolveAliasedPath() on those returns the home directory;

  2. We never anchored the lookup to a suitable path (ie, the user's home directory, or the parent of that directory)

This meant that opening a new file would trigger a recursive listing of all files within the home directory (and parent directory where permissions allowed), and I suspect this may be a very slow operation on certain NFS configurations.

This PR attempts to alleviate both issues.

This is a candidate for the v1.1 patch release, if we can confirm it does alleviate the slowness observed.

return fileNotFoundError(ERROR_LOCATION);

// bail if we're no longer within the anchor's parent path
if (!filePath.isWithin(anchorParentPath))
Copy link
Member

Choose a reason for hiding this comment

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

Not sure I understand this bit. How could we reach a file path that isn't within the anchor's parent without first traversing past the anchor itself?

If the intent is to bail out if the filePath is not within the original anchor, it seems like we could do that just once instead of inside the loop (let me know if I'm misreading!).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was aiming to capture situations where we might have something like the following:

anchor: /home/users/me
file:   /home/users/you/project/file.R

In the current scheme, we'd start testing the directories /home/users/you/project, /home/users/you, /home/users and /home for project files -- I wanted to ensure that we would stop once we hit /home/users in the above example. I'll see if I can express that more clearly.

@jmcphers
Copy link
Member

Thanks so much for tackling this one, here's hoping that it puts the problem behind us!

@jmcphers jmcphers merged commit 823a7bb into master Nov 16, 2017
@valerie-rstudio valerie-rstudio deleted the bugfix/windows-nfs-slowness branch January 21, 2022 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants