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.

Sign up for GitHub

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

mmap for windows #3557

Merged
merged 6 commits into from
Mar 28, 2023
Merged

mmap for windows #3557

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
48766b2
mmap for windows
daniellerozenblit Mar 17, 2023
3966167
remove enabling mmap for testing
daniellerozenblit Mar 17, 2023
ffc7bcf
rename FIO dictionary initialization methods + un-const dictionary ob…
daniellerozenblit Mar 27, 2023
104f7a5
remove enabling mmap for testing
daniellerozenblit Mar 27, 2023
5e79902
initDict returns void, underlying setDictBuffer methods return the si…
daniellerozenblit Mar 28, 2023
854f25f
fix comment
daniellerozenblit Mar 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 63 additions & 21 deletions programs/fileio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -693,10 +693,11 @@ static void FIO_getDictFileStat(const char* fileName, stat_t* dictFileStat) {
* @return : loaded size
* if fileName==NULL, returns 0 and a NULL pointer
*/
static size_t FIO_createDictBuffer(void** bufferPtr, const char* fileName, FIO_prefs_t* const prefs, stat_t* dictFileStat)
static size_t FIO_createDictBuffer(FIO_Dict_t* dict, const char* fileName, FIO_prefs_t* const prefs, stat_t* dictFileStat)
{
FILE* fileHandle;
U64 fileSize;
void** bufferPtr = &dict->dictBuffer;

assert(bufferPtr != NULL);
assert(dictFileStat != NULL);
Expand Down Expand Up @@ -733,20 +734,15 @@ static size_t FIO_createDictBuffer(void** bufferPtr, const char* fileName, FIO_p

#if (PLATFORM_POSIX_VERSION > 0)
#include <sys/mman.h>
static void* FIO_mmap(size_t fileSize, int fileHandle)
static int FIO_munmap(const FIO_Dict_t* dict)
daniellerozenblit marked this conversation as resolved.
Show resolved Hide resolved
{
return mmap
(NULL, (size_t)fileSize, PROT_READ, MAP_PRIVATE, fileHandle, 0);
return munmap(dict->dictBuffer, dict->dictBufferSize);
}
static int FIO_munmap(void* buffer, size_t bufferSize)
{
return munmap(buffer, bufferSize);
}
/* We might want to also do mapping for windows */
static size_t FIO_createDictBufferMMap(void** bufferPtr, const char* fileName, FIO_prefs_t* const prefs, stat_t* dictFileStat)
static size_t FIO_createDictBufferMMap(FIO_Dict_t* dict, const char* fileName, FIO_prefs_t* const prefs, stat_t* dictFileStat)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discussion on naming convention :

When I read a function name like PREFIX_createType(), I expect it to return a Type* pointer object.
That's not the case here.

On a related note, if the object is allocated beforehand (on stack for example), I would consider PREFIX_initType(Type* v, ...) for the function name.

I note that this situation is slightly different : you want to set member dict->dictBuffer with the outcome of mmap (and I presume FIO_Dict_t is larger than this member), and return the underlying file size.

In which case, maybe FIO_setDictBufferMMap() ?

{
int fileHandle;
U64 fileSize;
void** bufferPtr = &dict->dictBuffer;

assert(bufferPtr != NULL);
assert(dictFileStat != NULL);
Expand All @@ -770,27 +766,73 @@ static size_t FIO_createDictBufferMMap(void** bufferPtr, const char* fileName, F
}
}

*bufferPtr = FIO_mmap((size_t)fileSize, fileHandle);
*bufferPtr = mmap(NULL, (size_t)fileSize, PROT_READ, MAP_PRIVATE, fileHandle, 0);
daniellerozenblit marked this conversation as resolved.
Show resolved Hide resolved

close(fileHandle);
return (size_t)fileSize;
}
#elif defined(_MSC_VER) || defined(_WIN32)
#include <windows.h>
static int FIO_munmap(const FIO_Dict_t* dict)
daniellerozenblit marked this conversation as resolved.
Show resolved Hide resolved
{
UnmapViewOfFile(dict->dictBuffer);
return CloseHandle(dict->dictHandle);
}
static size_t FIO_createDictBufferMMap(FIO_Dict_t* dict, const char* fileName, FIO_prefs_t* const prefs, stat_t* dictFileStat)
{
HANDLE fileHandle, mapping;
U64 fileSize;
void** bufferPtr = &dict->dictBuffer;

assert(bufferPtr != NULL);
assert(dictFileStat != NULL);
*bufferPtr = NULL;
if (fileName == NULL) return 0;

DISPLAYLEVEL(4,"Loading %s as dictionary \n", fileName);

fileHandle = CreateFileA(fileName, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_READONLY, NULL);

if (fileHandle == INVALID_HANDLE_VALUE) {
EXM_THROW(33, "Couldn't open dictionary %s: %s", fileName, strerror(errno));
}

fileSize = UTIL_getFileSizeStat(dictFileStat);
{
size_t const dictSizeMax = prefs->patchFromMode ? prefs->memLimit : DICTSIZE_MAX;
if (fileSize > dictSizeMax) {
EXM_THROW(34, "Dictionary file %s is too large (> %u bytes)",
fileName, (unsigned)dictSizeMax); /* avoid extreme cases */
}
}

mapping = CreateFileMapping(fileHandle, NULL, PAGE_READONLY, 0, 0, NULL);
if (mapping == NULL) {
EXM_THROW(33, "Couldn't map dictionary %s: %s", fileName, strerror(errno));
}

*bufferPtr = MapViewOfFile(mapping, FILE_MAP_READ, 0, 0, (DWORD)fileSize); /* we can only cast to DWORD here because dictSize <= 2GB */
if (*bufferPtr == NULL) {
EXM_THROW(33, "Couldn't map dictionary %s: %s", fileName, strerror(errno));
}
dict->dictHandle = fileHandle;
return (size_t)fileSize;
}
#else
static size_t FIO_createDictBufferMMap(void** bufferPtr, const char* fileName, FIO_prefs_t* const prefs, stat_t* dictFileStat)
static size_t FIO_createDictBufferMMap(FIO_Dict_t* dict, const char* fileName, FIO_prefs_t* const prefs, stat_t* dictFileStat)
{
return FIO_createDictBuffer(bufferPtr, fileName, prefs, dictFileStat);
return FIO_createDictBuffer(dict, fileName, prefs, dictFileStat);
}
static void FIO_munmap(void* buffer, size_t bufferSize) {
(void)bufferSize;
free(buffer);
static void FIO_munmap(const FIO_Dict_t* dict) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment here, prefer FIO_Dict_t* dict as argument type.

free(dict->dictBuffer);
}
#endif

static void FIO_freeDict(const FIO_Dict_t* dict) {
if (dict->dictBufferType == FIO_mallocDict) {
free(dict->dictBuffer);
} else if (dict->dictBufferType == FIO_mmapDict) {
FIO_munmap(dict->dictBuffer, dict->dictBufferSize);
FIO_munmap(dict);
} else {
assert(0); /* Should not reach this case */
}
Expand Down Expand Up @@ -1068,9 +1110,9 @@ static cRess_t FIO_createCResources(FIO_prefs_t* const prefs,
ress.dict.dictBufferType = (useMMap && !forceNoUseMMap) ? FIO_mmapDict : FIO_mallocDict;

if (ress.dict.dictBufferType == FIO_mallocDict) {
ress.dict.dictBufferSize = FIO_createDictBuffer(&ress.dict.dictBuffer, dictFileName, prefs, &ress.dictFileStat); /* works with dictFileName==NULL */
ress.dict.dictBufferSize = FIO_createDictBuffer(&ress.dict, dictFileName, prefs, &ress.dictFileStat); /* works with dictFileName==NULL */
} else {
ress.dict.dictBufferSize = FIO_createDictBufferMMap(&ress.dict.dictBuffer, dictFileName, prefs, &ress.dictFileStat);
ress.dict.dictBufferSize = FIO_createDictBufferMMap(&ress.dict, dictFileName, prefs, &ress.dictFileStat);
}

ress.writeCtx = AIO_WritePool_create(prefs, ZSTD_CStreamOutSize());
Expand Down Expand Up @@ -2178,9 +2220,9 @@ static dRess_t FIO_createDResources(FIO_prefs_t* const prefs, const char* dictFi

/* dictionary */
{ if (ress.dict.dictBufferType == FIO_mallocDict) {
ress.dict.dictBufferSize = FIO_createDictBuffer(&ress.dict.dictBuffer, dictFileName, prefs, &statbuf);
ress.dict.dictBufferSize = FIO_createDictBuffer(&ress.dict, dictFileName, prefs, &statbuf);
} else {
ress.dict.dictBufferSize = FIO_createDictBufferMMap(&ress.dict.dictBuffer, dictFileName, prefs, &statbuf);
ress.dict.dictBufferSize = FIO_createDictBufferMMap(&ress.dict, dictFileName, prefs, &statbuf);
}

CHECK(ZSTD_DCtx_reset(ress.dctx, ZSTD_reset_session_only) );
Expand Down
3 changes: 3 additions & 0 deletions programs/fileio_types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ typedef struct {
void* dictBuffer;
size_t dictBufferSize;
FIO_dictBufferType_t dictBufferType;
#if defined(_MSC_VER) || defined(_WIN32)
HANDLE dictHandle;
#endif
} FIO_Dict_t;

#endif /* FILEIO_TYPES_HEADER */