This PR resolves a todo within the Win32LockedPageAllocator: // TODO is there a limit on Windows, how to get it?.
The idea is to use the Windows API to get the limits like the posix based allocator does with getrlimit.
I use GetProcessWorkingSetSize to perform this task and fallback to return std::numeric_limits<size_t>::max(); just like the posix implementation does.
201 | @@ -202,7 +202,11 @@ void Win32LockedPageAllocator::FreeLocked(void* addr, size_t len) 202 | 203 | size_t Win32LockedPageAllocator::GetLimit() 204 | { 205 | - // TODO is there a limit on Windows, how to get it? 206 | + size_t min, max; 207 | + if(GetProcessWorkingSetSize(GetCurrentProcess(), &min, &max) != 0)
Preferred code style:
if (GetProcessWorkingSetSize(GetCurrentProcess(), &min, &max) != 0) {
Concept ACK, thanks for filling this in.
The maximum number of pages that a process can lock is equal to the number of pages in its minimum working set minus a small overhead. - https://docs.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-virtuallock
Doesn't this mean we should return min, not max?
The maximum number of pages that a process can lock is equal to the number of pages in its minimum working set minus a small overhead.
Doesn't this mean we should return min, not max?
This may be my mistake, but I understood the function GetLimit to get the upper resource limit based on this comment: https://github.com/bitcoin/bitcoin/blob/a6cdc82313cc7cb2a8de8b99221719a5c8f52881/src/support/lockedpool.cpp#L345
I will change it along with the coding conventions.
There was a fuzzing step that the ci failed in, is this something I have to patch as well?
Thanks!
This may be my mistake, but I understood the function GetLimit to get the upper resource limit based on this comment:
The thing is that GetProcessWorkingSetSize returns the process working set size. From what i understand, this is the range from much program memory will be mapped into physical memory at minimum, to how many at maximum. According to that quote, the upper limit of what can be locked is the minimum working set size.
There was a fuzzing step that the ci failed in, is this something I have to patch as well?
I would guess it is unrelated.
The thing is that
GetProcessWorkingSetSizereturns the process working set size. From what i understand, this is the range from much program memory will be mapped into physical memory at minimum, to how many at maximum. According to that quote, the upper limit of what can be locked is the minimum working set size.
That makes sense, sorry for the confusion!
I would guess it is unrelated.
Oh great! Thanks for prompt replies.
Looks good to me now (but i can't test it)
maybe ping @sipsorcery
tACK 1cb42aeda37f4979923cd7e1c85febe994480de6.
Completely unrelated to this PR but someone obviously wasn't happy about one particular OS...
#ifndef WIN32
/** LockedPageAllocator specialized for OSes that don't try to be
* special snowflakes.
*/
Thank you @sipsorcery for taking the time to test it!
Completely unrelated to this PR but someone obviously wasn't happy about one particular OS...
LOL, i'm guilty of writing that. I was annoyed about all the special WIN32 exceptions. Mind that this was before MacOS became just as much, if not more, of a pain to support.