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

why do we need MAP_SHARED for h2o_buffer_reserve() #1585

Closed
chenbd opened this issue Dec 30, 2017 · 3 comments
Closed

why do we need MAP_SHARED for h2o_buffer_reserve() #1585

chenbd opened this issue Dec 30, 2017 · 3 comments

Comments

@chenbd
Copy link
Contributor

chenbd commented Dec 30, 2017

https://github.com/h2o/h2o/blob/master/lib/common/memory.c#L272

are there any others process which will access it? if not , why not set MAP_PRIVATE

also, can we use mremap(2) instead of mmap(2) and munmap(2) on platform which support mremap?

@kazuho
Copy link
Member

kazuho commented Dec 30, 2017

are there any others process which will access it? if not , why not set MAP_PRIVATE

The intent of using mktemps + fallocate + mmap is to create a file-backed buffer so that the maximum amount of space that can be used for buffering data would be constrained by the free space of the disk instead of the swap. MAP_PRIVATE conflicts with that goal.

also, can we use mremap(2) instead of mmap(2) and munmap(2) on platform which support mremap?

That would be possible assuming that the added pages will not be anonymous. Note also that if use of mremap shows a noticeable change in performance, then that would mean that we should see how to minimize the chance of pages being moved since unmapping (either permanently or as part of remaping) the mapped pages is the most expensive operation. And that does not necessarily mean that we should use mremap.

Anyways, buffering large amount of data is a secondary strategy and I do not think it is a good idea to spend effort on improving the performance. I would rather prefer working on adopting streaming request (see #1357) to handlers other than the proxy handler.

@chenbd
Copy link
Contributor Author

chenbd commented Jan 2, 2018

buffering large amount of data is a secondary strategy is the point.
guess the program rarely falls into mktemps + fallocate + mmap.

streaming request handled by handler which do not need large buffers, right?

@dyusupov
Copy link

dyusupov commented Jan 2, 2018

In my opinion usage of shared or private mmap resource (even as a temporary upload placeholder) is a bad idea. $TMP or $TMPDIR location can be anything in user environment, can be HDD for instance.. it is just an extra step for an admin to remember. Not to mention concurrent uploads of large files would easily create memory pressure if $TMP is ramdisk mount.

I very much like your suggestion and direction in getting streaming request work with non-proxy handlers. Looking forward and ready to test!

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

No branches or pull requests

3 participants