`fread` doesn't have to copy the file in memory even in the exceptional cases
Nobody has claimed this yet.
Assessment
- Difficulty
- 5/5
- Estimated time
- Over a week
- Newbie friendliness
- 30/100
- Issue type
- Feature
- Clarity
- Mostly clear
- Activity status
- Stale
- Domain
- operating-systems, performance
Research direction
Start in src/fread.c at the linked mappings and cleanup logic, then compare the POSIX mmap path with the Windows VirtualAlloc and MapViewOfFileEx approach described here. Test manually on Linux, Windows, macOS, FreeBSD, and another BSD. Done means the exceptional page-aligned cases avoid copying the whole file while mapping cleanup remains correct.
Written by the indexing model from the issue text.
Description
Currently, fread() copies the whole file into memory (in two cases) when it's an exact multiple of the memory page size and there is no place to insert a terminating \0 into the existing memory mapping. Here's how we can avoid it.
On POSIX-compatible operating systems, if (fileSize % sysconf(_SC_PAGE_SIZE) == 0) (not just 4096):
mmp = mmap(NULL, fileSize + sysconf(_SC_PAGE_SIZE), PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0). I don't think any reasonable OS with virtual memory will reserve physical memory in the RAM chips or swap space for a mapping with no access, but the virtual address space is now reserved; no othermmap()(ormalloc(), orallocVector()) will returnmmpuntil wemunmap()it.- Remember that the corresponding
munmap()call now needs to be forlength = fileSize + sysconf(_SC_PAGE_SIZE)as well, unconditionally. mmap(mmp, fileSize, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_FIXED, fd, 0). This replaces all but the last page of the mapping with the contents of the file.- Find out if the extra page is needed.
- If yes,
mmap((char*)mmp+fileSize, sysconf(_SC_PAGE_SIZE), PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0)). Now the last page is writable as well.
- If yes,
Windows makes the sensible decision of disallowing MAP_FIXED. It's almost always a bad idea to overwrite existing mappings like that; it's just that fread() as written is an exception that needs this feature to work well. On Windows, the following kludge can be used:
SYSTEM_INFO si; GetSystemInfo(&si);.if (fileSize % si.dwPageSize == 0):mmp = VirtualAlloc(NULL, fileSize + si.dwPageSize, MEM_RESERVE, 0);The operating system found a place in the virtual address space with enough space for the file contents plus one page of memory; it is now reserved against other allocations. The problem is, now we cannot map a file over it. So, quickly,VirtualFree(mmp, 0, MEM_RELEASE);remove the reserved pages andMapViewOfFileEx(hMap, FILE_MAP_COPY, 0, 0, fileSize, mmp)put the file mapping in their place.- Also quickly decide if we need an additional page after the file mapping.
- If we do,
VirtualAlloc((char*)mmp + fileSize, si.dwPageSize, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE);and remember to additionally callVirtualFree((char*)mmp + fileSize, 0, MEM_RELEASE)when we're done with the file mapping.
- If we do,
- This should have worked because
mmpwas positioned with enough virtual address space after it for the file mapping and an extra page, and R is single-threaded, so there should have been no competing memory allocations that could have taken away that address space after step (2). Still, if any of the steps (3) or (4) failed, restart from step (1).
The code will need to be tested manually at least on FreeBSD and some other BSD in addition to Linux+Windows+macOS. Maybe when we're less busy, this idea will interest someone enough to produce a PR. Maybe it's not worth the extra complexity and we'll close this issue.
- Dominant language
- R
- Stars
- 3.9k
- Forks
- 1.1k
- Avg merge
- 14h 4m
- Merged PRs (30d)
- 4
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from Rdatatable/data.table
-
as.data.table() recurses without end on a survival::Surv object (or any data.frame carrying one) Open
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
Rdatatable/data.table#7887 ·
-
consistency tests
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
Rdatatable/data.table#7853 · 3 comments ·
-
internals
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
Rdatatable/data.table#6938 · 1 comment ·
-
encoding fread
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
Rdatatable/data.table#5179 · 8 comments ·
-
documentation programming
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
Rdatatable/data.table#3199 · 3 comments ·
All issues in Rdatatable/data.table
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
r-lib/pkgdepends#485 · 3 comments ·
-
Difficulty 1/5 Under an hour Newbie friendliness 92/100
-
beginners blocker
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
-
enviPathR OpenBuild Error Build OK Build Warning policies-accepted pre-review precheck-passed
Difficulty 1/5 Under an hour Newbie friendliness 84/100
Bioconductor/BiocContributions#207 · 6 comments ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
datacarpentry/semester-biology#1255 ·