Remove Hard-Coded Restrictions to CodeCache Page Sizes on P #7795
Conversation
Instead of only accepting 64K and 16M as valid page sizes, accept any page size matching the valid size list from the OS. Except for 32-bit archs where 16M will only be enabled along with consolidatedCodeCache to save on address space. Signed-off-by: Luke Li <luke.li@ibm.com>
|
Running on pLinux with a HugePageSize of 2M, we used to have:
But now there will be no error message. |
|
FYI @zl-wang |
|
See also eclipse-openj9/openj9#22086 |
| || (SIXTY_FOUR_K == validPageSize) | ||
| || (SIXTEEN_M == validPageSize) | ||
| #else | ||
| #if !defined(OMR_ENV_DATA64) | ||
| BOOLEAN codeCacheConsolidationEnabled = FALSE; |
There was a problem hiding this comment.
this modification looks fine to me, since i am hesitating if we want to poke the 800-pound gorilla at this point. i did observe some inconsistent logic (intention rather, due to legacy code) re codeCacheConsolidation enablement:
here, we check for: env variable TR_ppcCodeCacheConsolidationEnabled
JIT side: we either check for env variable TR_CodeCacheConsolidation or OMR option TR_EnableCodeCacheConsolidation
as it is, probably nobody knows how to navigate these layers of checking web without reading the code.
There was a problem hiding this comment.
Can one env variable enable another env variable in the front end? If not, does that mean the user must always set all three envs for the code to work?
There was a problem hiding this comment.
two will do: 1) set two env var; or 2) set this one env var and set the option
There was a problem hiding this comment.
the option is set by default for 64bit though, but this env var is not applicable to 64bit anyway
There was a problem hiding this comment.
Do I need to change anything on this front then? Or is it fine to leave it as-is?
There was a problem hiding this comment.
leave it as it is for now. upon @dsouzai review, let's see if he wants to do something about this 32bit technical-debt area.
There was a problem hiding this comment.
another corner-case is (for 64bit): TR_DisableCodeCacheConsolidation + largePage request, it will take up a segment (256MB) virtual space for every codeCache allocation on AIX. but that is existing as it is.
There was a problem hiding this comment.
I think we leave the technical-debt cleanup for another PR. The current PR does what it describes; doing more will complicate matters. Best to just open an issue to track this so that the inevitable subjective discussions that happen when the cleanup occurs can happen elsewhere.
| /* Check if the TR_ppcCodeCacheConsolidationEnabled env variable is set. | ||
| * TR_ppcCodeCacheConsolidationEnabled is a manually set env variable used to indicate | ||
| * that Code Cache Consolidation is enabled in the JIT. | ||
| */ | ||
| BOOLEAN codeCacheConsolidationEnabled = FALSE; | ||
|
|
||
| if (OMRPORT_VMEM_MEMORY_MODE_EXECUTE == (OMRPORT_VMEM_MEMORY_MODE_EXECUTE & mode)) { | ||
| if (portLibrary->sysinfo_get_env(portLibrary, "TR_ppcCodeCacheConsolidationEnabled", NULL, 0) != -1) { | ||
| codeCacheConsolidationEnabled = TRUE; | ||
| } | ||
| } |
There was a problem hiding this comment.
same comment as for aix modifications.
|
jenkins build aarch64,arm,aix,plinux,riscv,xlinux,x32linux,zlinux |
Instead of only accepting 64K and 16M as valid page sizes, accept any page size matching the valid size list from the OS.
Except for 32-bit archs where 16M will only be enabled along with consolidatedCodeCache to save on address space.