Skip to content

win32: choose the page size as our value for the page size #3690

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

Merged
merged 1 commit into from
Mar 16, 2016

Conversation

carlosmn
Copy link
Member

We've been using the allocation granularity instead for this value,
but his refers to VirtualAlloc() rather than anything we do. Using this
as a value for the pool means every pool would become at least 64kB with
foreseeable results on memory pressure.

Do use the system's page size as our value for the page size.

@ethomson
Copy link
Member

Hmm. It's okay to change the page size that we use for pools, but mmap emulation is a bit stricter. We need to observe the allocation granularity for MapViewOfFile:

dwFileOffsetLow [in]
A low-order DWORD of the file offset where the view is to begin. The combination of the high and low offsets must specify an offset within the file mapping. They must also match the memory allocation granularity of the system. That is, the offset must be a multiple of the allocation granularity. To obtain the memory allocation granularity of the system, use the GetSystemInfo function, which fills in the members of a SYSTEM_INFO structure.

While often similar, these are not the same on Windows. We want to use the page
size on Windows for the pools, but for mmap we need to use the allocation
granularity as the alignment.

On the other platforms these values remain the same.
ethomson pushed a commit that referenced this pull request Mar 16, 2016
win32: choose the page size as our value for the page size
@ethomson ethomson merged commit de143ef into master Mar 16, 2016
@ethomson ethomson deleted the cmn/pool-limit branch January 9, 2019 10:20
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

Successfully merging this pull request may close these issues.

2 participants