util: modify Win32LockedPageAllocator to query windows for limit. #25320

1. 
   brokenprogrammer commented at 1:19 pm on June 9, 2022: contributor

   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.

2. fanquake added the label Windows on Jun 9, 2022
3. fanquake added the label Utils/log/libs on Jun 9, 2022
4. in src/support/lockedpool.cpp:206 in a6cdc82313 outdated

   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)
   

   ---

   ---

   

   laanwj commented at 3:10 pm on June 9, 2022:

   Preferred code style:

   0if (GetProcessWorkingSetSize(GetCurrentProcess(), &min, &max) != 0) {
   

5. 
   laanwj commented at 3:10 pm on June 9, 2022: member

   Concept ACK, thanks for filling this in.
6. 
   laanwj commented at 3:17 pm on June 9, 2022: member

   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?

7. 
   brokenprogrammer commented at 4:06 pm on June 9, 2022: contributor

   @laanwj

   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?

   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?

8. 
   laanwj commented at 4:24 pm on June 9, 2022: member

   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.

9. 
   brokenprogrammer commented at 4:29 pm on June 9, 2022: contributor

   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.

   That makes sense, sorry for the confusion!

   I would guess it is unrelated.

   Oh great! Thanks for prompt replies.

10. util: modify Win32LockedPageAllocator to query windows for limit 1cb42aeda3
11. brokenprogrammer force-pushed on Jun 10, 2022
12. 
    laanwj commented at 5:26 pm on June 10, 2022: member

    Looks good to me now (but i can’t test it)

    maybe ping @sipsorcery

13. 
    sipsorcery commented at 8:23 pm on June 12, 2022: member

    tACK 1cb42aeda37f4979923cd7e1c85febe994480de6.

    Completely unrelated to this PR but someone obviously wasn’t happy about one particular OS…

    0#ifndef WIN32
    1/** LockedPageAllocator specialized for OSes that don't try to be
    2 * special snowflakes.
    3 */
    

14. 
    brokenprogrammer commented at 4:43 am on June 13, 2022: contributor

    Thank you @sipsorcery for taking the time to test it!
15. 
    laanwj commented at 7:05 am on June 14, 2022: member

    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.

16. laanwj merged this on Jun 14, 2022
17. laanwj closed this on Jun 14, 2022

    ---

18. sidhujag referenced this in commit 7e2af8076d on Jun 15, 2022
19. DrahtBot locked this on Jun 14, 2023

Contributors
brokenprogrammer laanwj sipsorcery

Labels
Windows Utils/log/libs