ext/opcache: remove option huge_code_pages

MaxKellermann · MaxKellermann · commit 5b7b07d0a34b · 2023-02-07T17:28:28.000+01:00
This feature is useless at best, but is fragile and increases memory usage because it creates a copy of the PHP executable in RAM, when another copy is already in the kernel's page cache. Additionally, each PHP process has its own copy, multiplying the memory usage with the number of PHP processes. Even worse: under memory pressure, the kernel would usually just discard those code pages, and reload them from disk on demand; but with `huge_code_pages`, the kernel cannot simply discard those pages, instead it has to allocate room in the swap partition and then has to write the pages to the swap partition before being able to discard them. This adds disk I/O to an already extremely constrained situation. The feature was added 7 years ago in commit 669f0b3, and the commit message stated that this "provided 2% improvement on real-life apps, because of 2-3 times reduction in number of iTLB misses", but did not tell how these results were produced. I tried, but failed to measure a difference. None of my tests went any faster with `huge_code_pages` enabled. In a second attempt, I tried `perf stat` with events `itlb_misses.miss_causes_a_walk`, `itlb_misses.stlb_hit`, `itlb_misses.walk_completed`, `itlb_misses.walk_duration` and could not see any improvements here either. The results were unstable, but nothing suggested it was better with `huge_code_pages`. Quite contrary, this operation delays PHP startup by 5 ms (measured on an Intel Xeon E5-2630). phpinfo() with opcache.huge_code_pages=0: 17.92 msec task-clock:u # 0.985 CPUs utilized 29,804,809 cycles:u # 1.663 GHz 44,512,668 instructions:u # 1.49 insn per cycle phpinfo() with opcache.huge_code_pages=1: 23.20 msec task-clock:u # 0.988 CPUs utilized 35,709,593 cycles:u # 1.539 GHz 45,940,779 instructions:u # 1.29 insn per cycle In order to reduce PHP's memory size, reduce the load during PHP startup, simplify debugging and profiling, I propose removing this feature.
diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c
@@ -34,7 +34,6 @@
 #include "zend_vm.h"
 #include "zend_inheritance.h"
 #include "zend_exceptions.h"
-#include "zend_mmap.h"
 #include "zend_observer.h"
 #include "main/php_main.h"
 #include "main/SAPI.h"
@@ -2949,174 +2948,6 @@ static void accel_globals_ctor(zend_accel_globals *accel_globals)
 	memset(accel_globals, 0, sizeof(zend_accel_globals));
 }
 
-#ifdef HAVE_HUGE_CODE_PAGES
-# ifndef _WIN32
-#  include <sys/mman.h>
-#  ifndef MAP_ANON
-#   ifdef MAP_ANONYMOUS
-#    define MAP_ANON MAP_ANONYMOUS
-#   endif
-#  endif
-#  ifndef MAP_FAILED
-#   define MAP_FAILED ((void*)-1)
-#  endif
-#  ifdef MAP_ALIGNED_SUPER
-#   include <sys/types.h>
-#   include <sys/sysctl.h>
-#   include <sys/user.h>
-#   define MAP_HUGETLB MAP_ALIGNED_SUPER
-#  endif
-# endif
-
-# if defined(MAP_HUGETLB) || defined(MADV_HUGEPAGE)
-static zend_result accel_remap_huge_pages(void *start, size_t size, size_t real_size, const char *name, size_t offset)
-{
-	void *ret = MAP_FAILED;
-	void *mem;
-
-	mem = mmap(NULL, size,
-		PROT_READ | PROT_WRITE,
-		MAP_PRIVATE | MAP_ANONYMOUS,
-		-1, 0);
-	if (mem == MAP_FAILED) {
-		zend_error(E_WARNING,
-			ACCELERATOR_PRODUCT_NAME " huge_code_pages: mmap failed: %s (%d)",
-			strerror(errno), errno);
-		return FAILURE;
-	}
-	memcpy(mem, start, real_size);
-
-#  ifdef MAP_HUGETLB
-	ret = mmap(start, size,
-		PROT_READ | PROT_WRITE | PROT_EXEC,
-		MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED | MAP_HUGETLB,
-		-1, 0);
-#  endif
-	if (ret == MAP_FAILED) {
-		ret = mmap(start, size,
-			PROT_READ | PROT_WRITE | PROT_EXEC,
-			MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED,
-			-1, 0);
-		/* this should never happen? */
-		ZEND_ASSERT(ret != MAP_FAILED);
-#  ifdef MADV_HUGEPAGE
-		if (-1 == madvise(start, size, MADV_HUGEPAGE)) {
-			memcpy(start, mem, real_size);
-			mprotect(start, size, PROT_READ | PROT_EXEC);
-			munmap(mem, size);
-			zend_error(E_WARNING,
-				ACCELERATOR_PRODUCT_NAME " huge_code_pages: madvise(HUGEPAGE) failed: %s (%d)",
-				strerror(errno), errno);
-			return FAILURE;
-		}
-#  else
-		memcpy(start, mem, real_size);
-		mprotect(start, size, PROT_READ | PROT_EXEC);
-		munmap(mem, size);
-		zend_error(E_WARNING,
-			ACCELERATOR_PRODUCT_NAME " huge_code_pages: mmap(HUGETLB) failed: %s (%d)",
-			strerror(errno), errno);
-		return FAILURE;
-#  endif
-	}
-
-	// Given the MAP_FIXED flag the address can never diverge
-	ZEND_ASSERT(ret == start);
-	zend_mmap_set_name(start, size, "zend_huge_code_pages");
-	memcpy(start, mem, real_size);
-	mprotect(start, size, PROT_READ | PROT_EXEC);
-
-	munmap(mem, size);
-
-	return SUCCESS;
-}
-
-static void accel_move_code_to_huge_pages(void)
-{
-#if defined(__linux__)
-	FILE *f;
-	long unsigned int huge_page_size = 2 * 1024 * 1024;
-
-	f = fopen("/proc/self/maps", "r");
-	if (f) {
-		long unsigned int  start, end, offset, inode;
-		char perm[5], dev[10], name[MAXPATHLEN];
-		int ret;
-
-		while (1) {
-			ret = fscanf(f, "%lx-%lx %4s %lx %9s %ld %s\n", &start, &end, perm, &offset, dev, &inode, name);
-			if (ret == 7) {
-				if (perm[0] == 'r' && perm[1] == '-' && perm[2] == 'x' && name[0] == '/') {
-					long unsigned int  seg_start = ZEND_MM_ALIGNED_SIZE_EX(start, huge_page_size);
-					long unsigned int  seg_end = (end & ~(huge_page_size-1L));
-					long unsigned int  real_end;
-
-					ret = fscanf(f, "%lx-", &start);
-					if (ret == 1 && start == seg_end + huge_page_size) {
-						real_end = end;
-						seg_end = start;
-					} else {
-						real_end = seg_end;
-					}
-
-					if (seg_end > seg_start) {
-						zend_accel_error(ACCEL_LOG_DEBUG, "remap to huge page %lx-%lx %s \n", seg_start, seg_end, name);
-						accel_remap_huge_pages((void*)seg_start, seg_end - seg_start, real_end - seg_start, name, offset + seg_start - start);
-					}
-					break;
-				}
-			} else {
-				break;
-			}
-		}
-		fclose(f);
-	}
-#elif defined(__FreeBSD__)
-	size_t s = 0;
-	int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_VMMAP, getpid()};
-	long unsigned int huge_page_size = 2 * 1024 * 1024;
-	if (sysctl(mib, 4, NULL, &s, NULL, 0) == 0) {
-		s = s * 4 / 3;
-		void *addr = mmap(NULL, s, PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0);
-		if (addr != MAP_FAILED) {
-			if (sysctl(mib, 4, addr, &s, NULL, 0) == 0) {
-				uintptr_t start = (uintptr_t)addr;
-				uintptr_t end = start + s;
-				while (start < end) {
-					struct kinfo_vmentry *entry = (struct kinfo_vmentry *)start;
-					size_t sz = entry->kve_structsize;
-					if (sz == 0) {
-						break;
-					}
-					int permflags = entry->kve_protection;
-					if ((permflags & KVME_PROT_READ) && !(permflags & KVME_PROT_WRITE) &&
-					    (permflags & KVME_PROT_EXEC) && entry->kve_path[0] != '\0') {
-						long unsigned int seg_start = ZEND_MM_ALIGNED_SIZE_EX(start, huge_page_size);
-						long unsigned int seg_end = (end & ~(huge_page_size-1L));
-						if (seg_end > seg_start) {
-							zend_accel_error(ACCEL_LOG_DEBUG, "remap to huge page %lx-%lx %s \n", seg_start, seg_end, entry->kve_path);
-							accel_remap_huge_pages((void*)seg_start, seg_end - seg_start, seg_end - seg_start, entry->kve_path, entry->kve_offset + seg_start - start);
-							// First relevant segment found is our binary
-							break;
-						}
-					}
-					start += sz;
-				}
-			}
-			munmap(addr, s);
-		}
-	}
-#endif
-}
-# else
-static void accel_move_code_to_huge_pages(void)
-{
-	zend_error(E_WARNING, ACCELERATOR_PRODUCT_NAME ": opcache.huge_code_pages has no affect as huge page is not supported");
-	return;
-}
-# endif /* defined(MAP_HUGETLB) || defined(MADV_HUGEPAGE) */
-#endif /* HAVE_HUGE_CODE_PAGES */
-
 static int accel_startup(zend_extension *extension)
 {
 #ifdef ZTS
@@ -3148,16 +2979,6 @@ static int accel_startup(zend_extension *extension)
 	}
 #endif
 
-#ifdef HAVE_HUGE_CODE_PAGES
-	if (ZCG(accel_directives).huge_code_pages &&
-	    (strcmp(sapi_module.name, "cli") == 0 ||
-	     strcmp(sapi_module.name, "cli-server") == 0 ||
-		 strcmp(sapi_module.name, "cgi-fcgi") == 0 ||
-		 strcmp(sapi_module.name, "fpm-fcgi") == 0)) {
-		accel_move_code_to_huge_pages();
-	}
-#endif
-
 	/* no supported SAPI found - disable acceleration and stop initialization */
 	if (accel_find_sapi() == FAILURE) {
 		accel_startup_ok = false;
diff --git a/ext/opcache/ZendAccelerator.h b/ext/opcache/ZendAccelerator.h
@@ -186,9 +186,6 @@ typedef struct _zend_accel_directives {
 	bool      file_cache_consistency_checks;
 #if ENABLE_FILE_CACHE_FALLBACK
 	bool      file_cache_fallback;
-#endif
-#ifdef HAVE_HUGE_CODE_PAGES
-	bool      huge_code_pages;
 #endif
 	char *preload;
 #ifndef ZEND_WIN32
diff --git a/ext/opcache/config.m4 b/ext/opcache/config.m4
@@ -4,13 +4,6 @@ PHP_ARG_ENABLE([opcache],
     [Disable Zend OPcache support])],
   [yes])
 
-PHP_ARG_ENABLE([huge-code-pages],
-  [whether to enable copying PHP CODE pages into HUGE PAGES],
-  [AS_HELP_STRING([--disable-huge-code-pages],
-    [Disable copying PHP CODE pages into HUGE PAGES])],
-  [yes],
-  [no])
-
 PHP_ARG_ENABLE([opcache-jit],
   [whether to enable JIT],
   [AS_HELP_STRING([--disable-opcache-jit],
diff --git a/ext/opcache/jit/zend_jit_perf_dump.c b/ext/opcache/jit/zend_jit_perf_dump.c
@@ -52,12 +52,12 @@ extern unsigned int thr_self(void);
 /*
  * 1) Profile using perf-<pid>.map
  *
- * perf record php -d opcache.huge_code_pages=0 -d opcache.jit_debug=0x10 bench.php
+ * perf record php -d opcache.jit_debug=0x10 bench.php
  * perf report
  *
  * 2) Profile using jit-<pid>.dump
  *
- * perf record -k 1 php -d opcache.huge_code_pages=0 -d opcache.jit_debug=0x20 bench.php
+ * perf record -k 1 php -d opcache.jit_debug=0x20 bench.php
  * perf inject -j -i perf.data -o perf.data.jitted
  * perf report -i perf.data.jitted
  *
diff --git a/ext/opcache/tests/zzz_basic_logging.phpt b/ext/opcache/tests/zzz_basic_logging.phpt
@@ -9,7 +9,6 @@ opcache.enable_cli=1
 opcache.file_cache_only=0
 opcache.error_log=
 opcache.log_verbosity_level=4
-opcache.huge_code_pages=0
 opcache.preload=
 opcache.interned_strings_buffer=8
 --EXTENSIONS--
diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c
@@ -293,9 +293,6 @@ ZEND_INI_BEGIN()
 	STD_PHP_INI_BOOLEAN("opcache.file_cache_consistency_checks" , "1"   , PHP_INI_SYSTEM, OnUpdateBool,	   accel_directives.file_cache_consistency_checks, zend_accel_globals, accel_globals)
 #if ENABLE_FILE_CACHE_FALLBACK
 	STD_PHP_INI_BOOLEAN("opcache.file_cache_fallback"           , "1"   , PHP_INI_SYSTEM, OnUpdateBool,	   accel_directives.file_cache_fallback,           zend_accel_globals, accel_globals)
-#endif
-#ifdef HAVE_HUGE_CODE_PAGES
-	STD_PHP_INI_BOOLEAN("opcache.huge_code_pages"             , "0"   , PHP_INI_SYSTEM, OnUpdateBool,      accel_directives.huge_code_pages,               zend_accel_globals, accel_globals)
 #endif
 	STD_PHP_INI_ENTRY("opcache.preload"                       , ""    , PHP_INI_SYSTEM, OnUpdateStringUnempty,    accel_directives.preload,                zend_accel_globals, accel_globals)
 #ifndef ZEND_WIN32
@@ -807,9 +804,6 @@ ZEND_FUNCTION(opcache_get_configuration)
 	add_assoc_long(&directives,   "opcache.file_update_protection",  ZCG(accel_directives).file_update_protection);
 	add_assoc_long(&directives,   "opcache.opt_debug_level",         ZCG(accel_directives).opt_debug_level);
 	add_assoc_string(&directives, "opcache.restrict_api",            STRING_NOT_NULL(ZCG(accel_directives).restrict_api));
-#ifdef HAVE_HUGE_CODE_PAGES
-	add_assoc_bool(&directives,   "opcache.huge_code_pages",         ZCG(accel_directives).huge_code_pages);
-#endif
 	add_assoc_string(&directives, "opcache.preload", STRING_NOT_NULL(ZCG(accel_directives).preload));
 #ifndef ZEND_WIN32
 	add_assoc_string(&directives, "opcache.preload_user", STRING_NOT_NULL(ZCG(accel_directives).preload_user));
diff --git a/php.ini-development b/php.ini-development
@@ -1909,10 +1909,6 @@ ldap.max_links = -1
 ; cache is required.
 ;opcache.file_cache_fallback=1
 
-; Enables or disables copying of PHP code (text segment) into HUGE PAGES.
-; This should improve performance, but requires appropriate OS configuration.
-;opcache.huge_code_pages=0
-
 ; Validate cached file permissions.
 ;opcache.validate_permission=0
 
diff --git a/php.ini-production b/php.ini-production
@@ -1911,10 +1911,6 @@ ldap.max_links = -1
 ; cache is required.
 ;opcache.file_cache_fallback=1
 
-; Enables or disables copying of PHP code (text segment) into HUGE PAGES.
-; This should improve performance, but requires appropriate OS configuration.
-;opcache.huge_code_pages=1
-
 ; Validate cached file permissions.
 ;opcache.validate_permission=0
 

Original file line number	Diff line number	Diff line change
`@@ -52,12 +52,12 @@ extern unsigned int thr_self(void);`
`52`	`52`	`/*`
`53`	`53`	`* 1) Profile using perf-<pid>.map`
`54`	`54`	`*`
`55`		`- * perf record php -d opcache.huge_code_pages=0 -d opcache.jit_debug=0x10 bench.php`
	`55`	`+ * perf record php -d opcache.jit_debug=0x10 bench.php`
`56`	`56`	`* perf report`
`57`	`57`	`*`
`58`	`58`	`* 2) Profile using jit-<pid>.dump`
`59`	`59`	`*`
`60`		`- * perf record -k 1 php -d opcache.huge_code_pages=0 -d opcache.jit_debug=0x20 bench.php`
	`60`	`+ * perf record -k 1 php -d opcache.jit_debug=0x20 bench.php`
`61`	`61`	`* perf inject -j -i perf.data -o perf.data.jitted`
`62`	`62`	`* perf report -i perf.data.jitted`
`63`	`63`	`*`