zend alloc large page support for win(64) proposal. #8405
Conversation
|
cc @cmb69 |
| void *ptr; | ||
| #ifdef _WIN64 | ||
| // For now enabling only for 64 bits, seems more costly than benefitial otherwise | ||
| // ZEND_MM_CHUNK_SIZE being the minimum large page size too. |
There was a problem hiding this comment.
Are we sure about this? Don't we need to check GetLargePageMinimum()?
There was a problem hiding this comment.
I did in a small app, x86 64 supports 2MB and 1GB and GetLargePageMinimum returns the former.
There was a problem hiding this comment.
Yeah, but doesn't this depend on the processor? What if a processor doesn't support large pages at all, or requires a larger size than 2MiB (ZEND_MM_CHUNK_SIZE)?
There was a problem hiding this comment.
Fair point I ll add a check.
|
Generally, it would be good to do some performance testing for this with some real-world apps. |
Zend/zend_alloc.c
Outdated
| @@ -454,9 +454,13 @@ static void *zend_mm_mmap(size_t size) | |||
| #ifdef _WIN32 | |||
| void *ptr; | |||
| #ifdef _WIN64 | |||
| static zend_long minlgsz = -1; | |||
There was a problem hiding this comment.
A static variable won't play nicely with thread-safe builds. I think we need to initialize the variable in start_memory_manager(), so it won't be modified after startup.
devnexen
force-pushed
the
php_zend_win32_largepages
branch
2 times, most recently
from
b265427 to
01ea398
Compare
devnexen
force-pushed
the
php_zend_win32_largepages
branch
from
01ea398 to
ce3ae9c
Compare
|
I just checked this, and noticed that the large page allocation fails with code 1314 ("A required privilege is not held by the client.") According to the docs, we first need to
Apparently, it's starting to get "funny". |
|
ah good to know thanks, I may revisit it a bit later to see if this is worth pursuing, albeit I understand the reasoning behind this. |
@devnexen How is this looking? |
|
been sidetracked a bit but will be back to it soonish. |
some archs may have more than large page size supported but the minimum aligns with ZEND_MM_CHUNK_SIZE.
devnexen
force-pushed
the
php_zend_win32_largepages
branch
2 times, most recently
from
4237581 to
fd263ab
Compare
devnexen
force-pushed
the
php_zend_win32_largepages
branch
from
fd263ab to
460edaa
Compare
|
@cmb69 no rush but when you have a moment, still not sure about this. |
|
ping :) |
|
There has not been any recent activity in this PR. It will automatically be closed in 7 days if no further action is taken. |
|
We don't use huge pages for memory manager by default ( Adjusting process privileges from itself looks suspicious :) |
|
Looking at it now, I would need a real windows expert to double check anyway. |
some archs may have more than large page size supported but the minimum aligns with ZEND_MM_CHUNK_SIZE.