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

Speed up mount operation by putting winfsp network provider at the he… #131

Merged
merged 2 commits into from Jan 4, 2018
Merged

Speed up mount operation by putting winfsp network provider at the he… #131

merged 2 commits into from Jan 4, 2018

Conversation

felfert
Copy link
Contributor

@felfert felfert commented Dec 29, 2017

Hi @billziss-gh ,

This PR partially fixes #87 (which I'm experiencing as well).
The initial delay when mapping a network drive is gone with this fix.
In #87, you already mentioned putting the winfsp network provider entry at the beginning of the provider order instead of appending it. This PR does exactly that.
The other delays are unrelated (#87 actually is about two (or even more) different issues)
Tested on Win8.1

Cheers
-Fritz


Before submitting this PR please review this checklist. Ideally all checkmarks should be checked upon submitting. (Use an x inside square brackets like so: [x])

  • Contributing: You MUST read and be willing to accept the CONTRIBUTOR AGREEMENT. The agreement gives joint copyright interests in your contributions to you and the original WinFsp author. If you have already accepted the CONTRIBUTOR AGREEMENT you do not need to do so again.
  • Topic branch: Avoid creating the PR off the master branch of your fork. Consider creating a topic branch and request a pull from that. This allows you to add commits to the master branch of your fork without affecting this PR.
  • No tabs: Consistently use SPACES everywhere. NO TABS, unless the file format requires it (e.g. Makefile).
  • Style: Follow the same code style as the rest of the project.
  • [N/A] Tests: Include tests to the extent that it is possible, especially if you add a new feature.
  • Quality: Your design and code should be of high quality and something that you are proud of.

@billziss-gh
Copy link
Collaborator

@felfert

Thanks for the PR. Currently on vacation and unable to check it. Will test it on or after 2017-1-3.

@billziss-gh
Copy link
Collaborator

@felfert

I finally had the chance to look at this. I believe this should be accepted with a few changes.

  • I would like to have a preprocessor macro that controls whether the WinFsp.Np provider is added first or last in the list. The macro name should be FSP_NP_ORDER_FIRST and be defined by default. When the macro is defined the provider should be added first. When the macro is undefined the provider should be added last (as it is currently). This way we can easily revert this change, if we experience additional problems.

    /*
     * Define the following macro to register ourselves as the first network provider.
     * Otherwise we will be registered as the last network provider.
     */
    #define FSP_NP_ORDER_FIRST
  • You may be able to use the above conditional (FSP_NP_ORDER_FIRST) with RegBufferOffset=0 to avoid having too many ifdef's.

  • The line RegBufferOffset = lstrlenW(L"," FSP_NP_NAME); should probably be changed to the one below (note that the L"," is missing so that the two expressions should have the same value):

    RegBufferOffset = sizeof "" FSP_NP_NAME;

I also have a few comments:

  • In Long delay when opening properties of a network volume #87 you say the following:

    However the delays for displaying properties has increased now and it is existent for all directory levels, not only the toplevel.

    • Do you still observe this behavior?
  • In Long delay when opening properties of a network volume #87 you also say:

    If WinFsp.Np is last in the list, after a mount operation, a computer icon appears in the network neighborhood. Any action on that (expand, properties, etc.) fails after a long delay. If WinFsp.Np is first on the list, that computer icon does not get created. I have a feeling that the delays have something to do with this fake "computer" (even if it is not shown).

    • In my mind this actually argues for accepting your PR and making WinFsp.Np the first provider. My reasoning is that other network providers will not see non-existent "computer names" like such as "SSHFS".

@felfert
Copy link
Contributor Author

felfert commented Jan 4, 2018

Hi @billziss-gh,

I have added the FSP_NP_ORDER_FIRST macro and related changes.

In #87 you say the following:
However the delays for displaying properties has increased now and it is existent for all directory levels, not only the toplevel.

Do you still observe this behavior?

Nope. This was probably some confusion I had. Fact is however, that there are at least 2 different issues: The initial mount delay (fixed by this PR) and the secondary delay (approx 20sec) in explorer which occurs under the following conditions:

  • If the mounted drive is a WinFSP Network drive (UNC prefix is used)
  • Invocation of the Properties dialog anywhere in the hierarchy of the drive in question
  • Some cache has expired (wait ~ 30sec after the last Properties-Dialog invocation)

I was not able to reproduce the secondary delay programmatically or on the cmdline (like running attr .. for example). It happens only in explorer here. (Experience reliably on Win8.1/x64 as well as Win7/x86 with fuse-based code - did not test native FSP like memfs)

Cheers
-Fritz

@billziss-gh
Copy link
Collaborator

Fritz, thank you for your work. I will test and merge this in later tonight or tomorrow.

@felfert
Copy link
Contributor Author

felfert commented Jan 4, 2018

BTW:
@billziss-gh: It would be nice, if you could create a (beta?) release from that so one could test the new features (WiX-Depends, WorkDir, ProviderOrder)

Thanks
-Fritz

@billziss-gh billziss-gh merged commit 0c38f92 into winfsp:master Jan 4, 2018
@billziss-gh
Copy link
Collaborator

@felfert

I have accepted your PR. Thank you again for your quality work!

It would be nice, if you could create a (beta?) release from that so one could test the new features (WiX-Depends, WorkDir, ProviderOrder)

I will do so. I need to finish off a few other changes related to the launcher that I have in the works. I am hoping to have a new beta for you available by the weekend.

@billziss-gh
Copy link
Collaborator

@felfert

As per your request I have released WinFsp 2018.1 B1. Let me know if this works for you.

@felfert
Copy link
Contributor Author

felfert commented Jan 6, 2018

@billziss-gh

It works nicely. I have tested the new WorkDir registry value, the WiX-Depends feature and verified the provider order. The initial delay during mount is gone :-)
Furthermore, I might have found the root cause for the other delays of the "long delay" bug.
I'll write down some more observations there...

Cheers
-Fritz

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

Successfully merging this pull request may close these issues.

Long delay when opening properties of a network volume
2 participants