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

File resize on munmap #5

Open
RoyBellingan opened this issue Dec 1, 2019 · 2 comments
Open

File resize on munmap #5

RoyBellingan opened this issue Dec 1, 2019 · 2 comments

Comments

@RoyBellingan
Copy link

RoyBellingan commented Dec 1, 2019

Amazing library!
But I can suggest a pair of improvement.

I founded written no where that the file to be mapped need to be at least 1 byte, no big deal, but It took a bit to discovery.
If you do a
auto f = fopen("testfile", "w+");
auto fNo = fileno(f);
ftruncate(fNo,0);
using namespace mmap_allocator_namespace;
mmappable_vector<int, mmap_allocator > v =
mmappable_vector<int, mmap_allocator >(mmap_allocator("testfile", READ_WRITE_SHARED, 0, ALLOW_REMAP));

for (int var = 0; var < 50; ++var) {
	v.push_back(var);
}

It will crash, I have no idea why this happen...

The second suggestion is when using mmap_allocator("testfile", READ_WRITE_SHARED, 0, ALLOW_REMAP)
The file should be ftruncate to the size of the mmaped region, or else the remaining part will be lost.

Due to having multiple mmaped region, I have no idea how to perform this truncate on the fly, If I find I will do a pull request (of course out of the library is just a ftruncate(file,vector.size() * sizeof (int));)

@RoyBellingan
Copy link
Author

RoyBellingan commented Dec 1, 2019

Just discovered that even if I initialize the file to 1byte, if the vector grow over 1024, the same error will occour...
I do not know after which treshold and why, but the vector grow and realloc internally doubling it's size each time, so even if the size is "correct" it will stil fail for example in case of

ftruncate(fNo, 5000 * sizeof(int));

for (int var = 0; var < 5000; ++var) {
	v.push_back(var);
}

@johannesthoma
Copy link
Owner

Hi Roy, at the moment mmap_allocator does not support resizing the vector. It
is meant mainly for reading files that are already on disk. Supporting resizing
should be possible (via mremap()) however at the moment I am busy with
WinDRBD (www.github.com/Linbit/WinDRBD) ... If you wish to implement
resizing then please do, patches welcome ;)

The 1024 ints limit is easily explained by looking at PAGE_SIZE (which is
4KB on your system), mmap() uses pages to map the memory therefore
one can add some elements at the end of the vector if the last page
is not used completely.

The empty file bug definitately has to be fixed, I will try to find some time
and implement that in the next weeks.

Thanks for you input and for using mmap_allocator.

  • Johannes

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

2 participants