Enhance moving PHP code pages into huge pages #12806

PeterYang12 · 2023-11-28T02:48:00Z

The former implementation of huge pages for PHP code segments may fail to map in certain scenarios. For instance, if the initial 'r-x-' segment is not PHP, it will result in a failure to map into huge pages. Consequently, the optimization for huge pages with PHP code will no longer be effective.

This patch improves the implementation by accurately matching all 'r-x-' segments until it locates the PHP code segment. Subsequently, it performs the necessary huge page mapping.

Signed-off-by: PeterYang12 yuhan.yang@intel.com
Reviewed-by: chen-hu-97 hu1.chen@intel.com

PeterYang12 · 2023-11-28T02:57:12Z

For example:
If use this command to start php-fpm /lib64/ld-linux-x86-64.so.2 /usr/local/sbin/php-fpm , the older implementation for huge pages will not take effect.
ld-linux-x86-64.so.2 will pack all the binaries into 4GB memories, like this:

7ffff336c000-7ffff33ec000 rwxp 00000000 00:00 0
...
7ffff3451000-7ffff3454000 r--p 00000000 00:d8 18389528                   /lib/x86_64-linux-gnu/libnss_files-2.28.so
7ffff3454000-7ffff345b000 r-xp 00003000 00:d8 18389528                   /lib/x86_64-linux-gnu/libnss_files-2.28.so
...
7ffff6746000-7ffff6836000 r--p 00000000 00:d8 32576869                   /usr/local/sbin/php-fpm
7ffff6836000-7ffff6946000 ---p 000f0000 00:d8 32576869                   /usr/local/sbin/php-fpm
7ffff6946000-7ffff6c88000 r-xp 00200000 00:d8 32576869                   /usr/local/sbin/php-fpm
7ffff6c88000-7ffff6d46000 ---p 00542000 00:d8 32576869                   /usr/local/sbin/php-fpm
...
7ffff7ffe000-7ffff81fc000 rw-p 00000000 00:00 0                          [heap]
7ffff81fc000-7ffff8256000 rw-p 00000000 00:00 0                          [heap]

PeterYang12 · 2023-11-28T03:02:30Z

This patch should fix #12807

dstogov

The patch looks good. Only minor changes requested and few minor questions.

ext/opcache/ZendAccelerator.c

dstogov · 2023-11-28T07:48:02Z

ext/opcache/ZendAccelerator.c

+			if (ret >= 6) {
+				if (name[0] == '[') {


In sscanf() reads 6 arguments value of name may be undefined.

Same reason above. The name value in /proc/self/maps may be empty, I use >=6 to do the judgement.

The fact that name value in /ptoc/self/maps is empty doesn't mean that the value of variable name becomes empty. If scanf() processed only 6 variables, the value of name is undefined (most probably unchanged).

Sorry for my wrong explanation. In the codes, scanf() will processe 7 variables:

ret=sscanf(buffer, "%lx-%lx %4s %lx %9s %lu %s\n", &start, &end, perm, &offset, dev, &inode, name);

If the name in maps is empty, it only return 6 but not 7.

Yes, but the code below checks variable name even if ret == 6. I suppose lines without name should be skipped.

OK. Changed the codes and still use ret==7 to do the judgement. Thank you~

dstogov · 2023-11-28T07:49:29Z

ext/opcache/ZendAccelerator.c

+			ret = sscanf(buffer, "%lx-%lx %4s %lx %9s %lu %s\n", &start, &end, perm, &offset, dev, &inode, name);
+			if (ret >= 6) {
+				if (name[0] == '[') {
+                    // meet heap segment, should break out.


Why do you stop on [heap] segment?
We stop on program code segment, but I'm not sure if [heap] may be placed before it.

I saw most of the maps, [heap] segment is under program code segment in maps. So I suppose if met heap, the huge page mapping operation should break . Do you think it's unnecessary?

I think, it's better to remove this. It's not a problem to scan until the end of the file.

Thank you. I will remove this.

dstogov · 2023-11-28T07:54:58Z

ext/opcache/ZendAccelerator.c

 				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;
-					}
+					char *is_substring = strstr(name, __progname);
+					// try to find the php text segment and map it into huge pages
+					if (is_substring != NULL) {


Can you please minimize the diff avoiding unnecessary indentation changes.

- if (perm[0] == 'r' && perm[1] == '-' && perm[2] == 'x' && name[0] == '/') { + if (perm[0] == 'r' && perm[1] == '-' && perm[2] == 'x' && name[0] == '/' + && strstr(name, __progname) != NULL) { ... (the rest seems unchanged)

Done, thank you for your review. :)

dstogov · 2023-11-28T10:41:58Z

ext/opcache/ZendAccelerator.c

 			if (ret == 7) {
-				if (perm[0] == 'r' && perm[1] == '-' && perm[2] == 'x' && name[0] == '/') {
+				// try to find the php text segment and map it into huge pages
+				char *is_substring = strstr(name, __progname);
+				if (perm[0] == 'r' && perm[1] == '-' && perm[2] == 'x' && name[0] == '/' && is_substring != NULL) {


I would rewrite this like

if (ret >= 6) { if (ret > 6 && perm[0] == 'r' && perm[1] == '-' && perm[2] == 'x' && name[0] == '/' && strstr(name, __progname)) {

lines without "name" are going to be skipped and won't stop the search

strstr() call is going to be performed only after the cheaper checks

Done. Thank you for your kindness help~

The former implementation of huge pages for PHP code segments may fail to map in certain scenarios. For instance, if the initial 'r-x-' segment is not PHP, it will result in a failure to map into huge pages. Consequently, the optimization for huge pages with PHP code will no longer be effective. This patch improves the implementation by accurately matching all 'r-x-' segments until it locates the PHP code segment. Subsequently, it performs the necessary huge page mapping. Signed-off-by: PeterYang12 <yuhan.yang@intel.com> Reviewed-by: chen-hu-97 <hu1.chen@intel.com>

PeterYang12 requested review from dstogov and iluuu1994 as code owners November 28, 2023 02:48

github-actions bot added the Extension: opcache label Nov 28, 2023

dstogov requested changes Nov 28, 2023

View reviewed changes

PeterYang12 force-pushed the enhance-hugepage-for-php-text branch 2 times, most recently from 02d60f7 to 562fcaf Compare November 28, 2023 10:16

dstogov reviewed Nov 28, 2023

View reviewed changes

PeterYang12 force-pushed the enhance-hugepage-for-php-text branch from 562fcaf to ced9c52 Compare November 29, 2023 01:28

dstogov merged commit 566100d into php:master Nov 29, 2023

Enhance moving PHP code pages into huge pages #12806

Enhance moving PHP code pages into huge pages #12806

Uh oh!

Conversation

PeterYang12 commented Nov 28, 2023

Uh oh!

PeterYang12 commented Nov 28, 2023

Uh oh!

PeterYang12 commented Nov 28, 2023

Uh oh!

dstogov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PeterYang12 Nov 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PeterYang12 Nov 29, 2023 •

edited

Loading