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

WasmFS: Support non-private mmap #17801

Open
tlively opened this issue Sep 6, 2022 · 4 comments
Open

WasmFS: Support non-private mmap #17801

tlively opened this issue Sep 6, 2022 · 4 comments
Assignees
Labels

Comments

@tlively
Copy link
Member

tlively commented Sep 6, 2022

The _mmap_js implementation in syscalls.cpp has a TODO:

  // TODO: handle non-private mmaps. Those can be optimized in interesting ways
  //       like avoiding an allocation and a copy as we do below (whereas a
  //       private mmap is always a copy into a new, private region not shared
  //       with anything else).

It also asserts that the flags contain MAP_PRIVATE. We don't need to fully address the optimizations in the TODO yet, but we do need to do better than asserting on MAP_SHARED.

@tlively tlively added this to the WasmFS feature parity milestone Sep 6, 2022
@tlively tlively moved this to Todo in WasmFS Sep 6, 2022
@kripken
Copy link
Member

kripken commented Sep 6, 2022

We could support MAP_SHARED on MemoryFile in the simple case where it is not reallocated. That is basically what we supported in the old JS FS. Anything more would probably be quite hard, though.

Before working on this I'll try to collect data on how useful the limited form is. I'm not sure it's been used much in the old FS - we've normally recommended people to not use MAP_SHARED at all (and use features we can fully support).

@past-due
Copy link
Contributor

It looks like MAP_SHARED is used by __map_file, which is used by musl's gettext / libintl implementation:

if ((map = __map_file(name, &map_size))) break;

So it is needed to load message catalogs.

@tlively
Copy link
Member Author

tlively commented Sep 22, 2022

Good find, @past-due! It looks like the implementation of __map_file also closes the file descriptor right after mapping, so in the current implementation that ties mappings to fds, there's no way that modifications can make it back to the file.

@tlively
Copy link
Member Author

tlively commented Feb 2, 2023

A couple users ran into mmap bugs on #15140.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Todo
Development

No branches or pull requests

3 participants