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

Optimization: Keep some requests between GC. #120

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nyaxt
Copy link

@nyaxt nyaxt commented Jan 16, 2016

Allocating buffer of bufSize is typically large enough to kick off GC, and GC flushes all pooled messages.
To avoid too much GC kicking in, let 2 messages survive between GC.

This change makes https://github.com/nyaxt/otaru faster by 30%.

@bufdev
Copy link

bufdev commented Jan 29, 2016

@tv42 I have to second this one, big time. For comparison, I wrote a "reference" fuse implementation that is effectively a pass-through to the OS file system, and I've only done extremely simple benchmarks, but using this fork makes write performance approximately 2x faster.

@nyaxt furthermore, I tested values of minPooledReq by powers of two with some basic memory profiling. With the current bazil.org/fuse implementation, 99.5.% of allocated memory (-alloc_space) is in allocMessage in these benchmarks. With this fork, it drops to about 97.5%. But if you change minPooledReq to 32, it drops to about 53%, and there's a performance benefit. The benefits drop off after 32 (ie 64 isn't really better, 128 is not).

This might indicate a more overall design discussion, but just of note:

  1. Yes, please merge this :)
  2. Maybe we should discuss the overall design?

I'm going to send up the dummy implementation as a separate PR.

@tv42
Copy link
Member

tv42 commented Jun 24, 2016

Sorry for being too silent. I'm looking at very similar problems right now, and realized this probably isn't prominent enough:

Long-term plan for performance, as far as Linux is concerned, is vmsplice: #35

That gets us rid of managing per-request buffers completely.

@tv42
Copy link
Member

tv42 commented Jun 24, 2016

9e8e652 should help here. Leaving this pull open until I've actually had time to experiment on whether it's useful even after the decreased gc churn.

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.

3 participants