`fread` doesn't have to copy the file in memory even in the exceptional cases

Open
#7,191 2 comments 1 reaction 0 assignees View on GitHub

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
Tech stack
c, r

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

enhancement fread Low

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):

  1. 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 other mmap() (or malloc(), or allocVector()) will return mmp until we munmap() it.
  2. Remember that the corresponding munmap() call now needs to be for length = fileSize + sysconf(_SC_PAGE_SIZE) as well, unconditionally.
  3. 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.
  4. 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.

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:

  1. SYSTEM_INFO si; GetSystemInfo(&si);. if (fileSize % si.dwPageSize == 0):
  2. 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,
  3. VirtualFree(mmp, 0, MEM_RELEASE); remove the reserved pages and
  4. MapViewOfFileEx(hMap, FILE_MAP_COPY, 0, 0, fileSize, mmp) put the file mapping in their place.
  5. 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 call VirtualFree((char*)mmp + fileSize, 0, MEM_RELEASE) when we're done with the file mapping.
  6. This should have worked because mmp was 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from Rdatatable/data.table

All issues in Rdatatable/data.table

Similar issues

More R issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.