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

Much faster with logging disabled #38

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

PoeticDeath
Copy link

When the logger is disabled the strings to be logged were still being constructed. The speed difference mainly noticeable with reads, are due to each read byte being logged. Should not adversely affect logging or consistency. On the left is the speed before, while right is the speed after the patch.
Speed

When the logger is disabled the strings to be logged were still being constructed. The speed difference mainly noticeable with reads, are due to each read byte being logged. Should not adversely affect logging or consistency.
@touilleMan
Copy link
Member

Hi @PoeticDeath, and thanks for this PR ;-)

I can see there is two different things there:

  1. remove logging overhead when it is not needed
  2. improve handling of buffers used in get_reparse_point

Can you explain the performance gain comes from 1), 2) or both

Also what is the need for 2) (it seems like get_reparse_point only raises NotImplementedError right now

Finally while better performances is always good, from what I understand here your PR only improve the performances of the memfs demo

9 similar comments
@touilleMan
Copy link
Member

Hi @PoeticDeath, and thanks for this PR ;-)

I can see there is two different things there:

  1. remove logging overhead when it is not needed
  2. improve handling of buffers used in get_reparse_point

Can you explain the performance gain comes from 1), 2) or both

Also what is the need for 2) (it seems like get_reparse_point only raises NotImplementedError right now

Finally while better performances is always good, from what I understand here your PR only improve the performances of the memfs demo

@touilleMan
Copy link
Member

Hi @PoeticDeath, and thanks for this PR ;-)

I can see there is two different things there:

  1. remove logging overhead when it is not needed
  2. improve handling of buffers used in get_reparse_point

Can you explain the performance gain comes from 1), 2) or both

Also what is the need for 2) (it seems like get_reparse_point only raises NotImplementedError right now

Finally while better performances is always good, from what I understand here your PR only improve the performances of the memfs demo

@touilleMan
Copy link
Member

Hi @PoeticDeath, and thanks for this PR ;-)

I can see there is two different things there:

  1. remove logging overhead when it is not needed
  2. improve handling of buffers used in get_reparse_point

Can you explain the performance gain comes from 1), 2) or both

Also what is the need for 2) (it seems like get_reparse_point only raises NotImplementedError right now

Finally while better performances is always good, from what I understand here your PR only improve the performances of the memfs demo

@touilleMan
Copy link
Member

Hi @PoeticDeath, and thanks for this PR ;-)

I can see there is two different things there:

  1. remove logging overhead when it is not needed
  2. improve handling of buffers used in get_reparse_point

Can you explain the performance gain comes from 1), 2) or both

Also what is the need for 2) (it seems like get_reparse_point only raises NotImplementedError right now

Finally while better performances is always good, from what I understand here your PR only improve the performances of the memfs demo

@touilleMan
Copy link
Member

Hi @PoeticDeath, and thanks for this PR ;-)

I can see there is two different things there:

  1. remove logging overhead when it is not needed
  2. improve handling of buffers used in get_reparse_point

Can you explain the performance gain comes from 1), 2) or both

Also what is the need for 2) (it seems like get_reparse_point only raises NotImplementedError right now

Finally while better performances is always good, from what I understand here your PR only improve the performances of the memfs demo

@touilleMan
Copy link
Member

Hi @PoeticDeath, and thanks for this PR ;-)

I can see there is two different things there:

  1. remove logging overhead when it is not needed
  2. improve handling of buffers used in get_reparse_point

Can you explain the performance gain comes from 1), 2) or both

Also what is the need for 2) (it seems like get_reparse_point only raises NotImplementedError right now

Finally while better performances is always good, from what I understand here your PR only improve the performances of the memfs demo

@touilleMan
Copy link
Member

Hi @PoeticDeath, and thanks for this PR ;-)

I can see there is two different things there:

  1. remove logging overhead when it is not needed
  2. improve handling of buffers used in get_reparse_point

Can you explain the performance gain comes from 1), 2) or both

Also what is the need for 2) (it seems like get_reparse_point only raises NotImplementedError right now

Finally while better performances is always good, from what I understand here your PR only improve the performances of the memfs demo

@touilleMan
Copy link
Member

Hi @PoeticDeath, and thanks for this PR ;-)

I can see there is two different things there:

  1. remove logging overhead when it is not needed
  2. improve handling of buffers used in get_reparse_point

Can you explain the performance gain comes from 1), 2) or both

Also what is the need for 2) (it seems like get_reparse_point only raises NotImplementedError right now

Finally while better performances is always good, from what I understand here your PR only improve the performances of the memfs demo

@touilleMan
Copy link
Member

Hi @PoeticDeath, and thanks for this PR ;-)

I can see there is two different things there:

  1. remove logging overhead when it is not needed
  2. improve handling of buffers used in get_reparse_point

Can you explain the performance gain comes from 1), 2) or both

Also what is the need for 2) (it seems like get_reparse_point only raises NotImplementedError right now

Finally while better performances is always good, from what I understand here your PR only improve the performances of the memfs demo

@PoeticDeath
Copy link
Author

I've been developing my own filesystem currently open source, named SpaceFS. Refuse, another module for use of WinFsp in Python, is great for compatibility between major OSs. But I wanted Windows specific functions to be supported where possible. Thus I looked for another solution and found your winfspy.

However almost immediately had found some performance issues. Using kernprof, a line by line profiler. I narrowed down a good chunk to the creation of debug strings even when it is disabled. To avoid this I added a simple if check of logging.root.level seen with in the first commit. This improvement will help any code or filesystem that uses your logging function.

Since then I have focused on properly implementing features, I managed to get pretty much everything to work correctly testable through WinFsp's winfsp-test. However when I got to reparse handing it was not implemented nearly at all, neither was GetStreamInfo. So I have worked on properly implementing them as well, GetStreamInfo needs some work still, many filenames will be reported correctly while a few others will not, but reparse seems to be complete. This should have little or no affect to performance, just completeness.

Sorry it took so long to respond for whatever reason GitHub did not notify me to your comment. Looking forward to working with you.

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.

2 participants