# Marking error, etc. functions as cold? - Thread: https://externals.io/message/87761 - Original archive: https://news-web.php.net/php.internals/87761 - Started by Matt Wilmas on 2015-08-17T10:26:30+00:00 - Messages: 4 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #87761 — Matt Wilmas — 2015-08-17T10:26:30+00:00 URL: https://externals.io/message/87761#87761 Hi Dmitry, all, Has it been considered to use __attribute__((cold)) on, for example, error-type functions? I happened to notice this part about it in the GCC docs [1] a couple days ago: "The paths leading to calls of cold functions within code are marked as unlikely by the branch prediction mechanism." A while ago I figured that, technically, many if () conditions for errors could use UNEXPECTED(), but that would be overkill for each possible tiny advantage. But that sort of happens automatically for conditions leading to "cold" functions! :^) I was thinking of widely-used functions like php_error_docref*, zend_error (and related); also noticed exception/throwing functions, or even zend_accel_error in opcache. I didn't do any profiling, just simply tried marking *only* php_error_docref0 cold. With GCC 4.8, it reduced the size by almost 4 KB (-O2, --disable-all CLI). And checking through the code, it did indeed move the calls around (e.g. out of fast path) in many cases compared to default (some cases didn't optimize more). Looks like more zend_error calls in VM that aren't already part of UNEXPECTED() conditions could be moved "out of the way" if marked cold... [1] https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html Thoughts? Thanks, Matt --- ## #87765 — Dmitry Stogov — 2015-08-17T11:38:33+00:00 URL: https://externals.io/message/87761#87765 · In reply to: #87761 This is good idea. Some times ago I played with "hot" attribute, and didn't get any improvement, but we really may get something with "cold". Thanks. Dmitry. On Mon, Aug 17, 2015 at 1:26 PM, Matt Wilmas wrote: > Hi Dmitry, all, > > Has it been considered to use __attribute__((cold)) on, for example, > error-type functions? I happened to notice this part about it in the GCC > docs [1] a couple days ago: "The paths leading to calls of cold functions > within code are marked as unlikely by the branch prediction mechanism." > > A while ago I figured that, technically, many if () conditions for errors > could use UNEXPECTED(), but that would be overkill for each possible tiny > advantage. But that sort of happens automatically for conditions leading > to "cold" functions! :^) > > I was thinking of widely-used functions like php_error_docref*, zend_error > (and related); also noticed exception/throwing functions, or even > zend_accel_error in opcache. > > I didn't do any profiling, just simply tried marking *only* > php_error_docref0 cold. With GCC 4.8, it reduced the size by almost 4 KB > (-O2, --disable-all CLI). And checking through the code, it did indeed > move the calls around (e.g. out of fast path) in many cases compared to > default (some cases didn't optimize more). > > Looks like more zend_error calls in VM that aren't already part of > UNEXPECTED() conditions could be moved "out of the way" if marked cold... > > [1] https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html > > Thoughts? > > > Thanks, > Matt --- ## #87797 — Matt Wilmas — 2015-08-19T13:05:35+00:00 URL: https://externals.io/message/87761#87797 · In reply to: #87765 Hi Dmitry, ----- Original Message ----- From: "Dmitry Stogov" Sent: Monday, August 17, 2015 > This is good idea. > Some times ago I played with "hot" attribute, and didn't get any > improvement, but we really may get something with "cold". It was cool to hear that. :-) I just noticed the changes you commited, http://git.php.net/?p=php-src.git;a=commitdiff;h=71af54e5f6656af070e4dccd1470bb1376c89568 What were your findings if you investigated deeper? I later checked with Cachegrind, and after I first only tried "cold" on php_error_docref0, there were 85,000 more branch mispredicts on bench.php (hmm, and I didn't think it really used PHP functions much). But when I also did zend_error[_noreturn], there was 20,000 fewer instructions (?) and 300,000 fewer mispredicts (0.16%), so a distinct improvement. :-) So what about php_error_docref* stuff...? Or any non-Zend functions. I had only applied cold to the function declaration (not definition), since that's what the docs said about function attributes [1]... The macros: I noticed you made HOT cold and wondered about typo. :-) Your fix only fixed it for the *non-GCC* #define, and I was going to mention that those should be empty anyway! OK, see you just fixed that. ;-) BTW, the ZEND_ATTRIBUTE_UNUSED[_LABEL] ones aren't used anywhere -- leftover from the initial FAST_ZPP patch. And speaking of typos: line 80 of zend.h has "#ifdef ZEND_NORETRUN_ALIAS" instead of NORETURN. [1] https://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html > Thanks. Dmitry. Thanks, Matt > On Mon, Aug 17, 2015 at 1:26 PM, Matt Wilmas > wrote: > > > […] --- ## #87808 — Dmitry Stogov — 2015-08-19T15:57:43+00:00 URL: https://externals.io/message/87761#87808 · In reply to: #87797 Hi Matt, On Wed, Aug 19, 2015 at 4:05 PM, Matt Wilmas wrote: > Hi Dmitry, > > ----- Original Message ----- > From: "Dmitry Stogov" > Sent: Monday, August 17, 2015 > > This is good idea. > > […] > > It was cool to hear that. :-) I just noticed the changes you commited, > http://git.php.net/?p=php-src.git;a=commitdiff;h=71af54e5f6656af070e4dccd1470bb1376c89568 yeah, I got some time to try your idea :) > > What were your findings if you investigated deeper? I later checked with > Cachegrind, and after I first only tried "cold" on php_error_docref0, there > were 85,000 more branch mispredicts on bench.php (hmm, and I didn't think > it really used PHP functions much). But when I also did > zend_error[_noreturn], there was 20,000 fewer instructions (?) and 300,000 > fewer mispredicts (0.16%), so a distinct improvement. :-) > This doesn't make visible speed improvement for me, but even code size reduction is good. > So what about php_error_docref* stuff...? Or any non-Zend functions. > Ut may make sense to add COLD attribute for some other function. > > I had only applied cold to the function declaration (not definition), > since that's what the docs said about function attributes [1]... > For some internal functions we just don't have declarations. Anyway, the patch works as expected. > > The macros: I noticed you made HOT cold and wondered about typo. :-) Your > fix only fixed it for the *non-GCC* #define, and I was going to mention > that those should be empty anyway! OK, see you just fixed that. ;-) > > BTW, the ZEND_ATTRIBUTE_UNUSED[_LABEL] ones aren't used anywhere -- > leftover from the initial FAST_ZPP patch. > yeah. it may be useful anyway. > > And speaking of typos: line 80 of zend.h has "#ifdef ZEND_NORETRUN_ALIAS" > instead of NORETURN. > ops. thanks. I'll fix it. Thanks. Dmitry. > > [1] https://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html > > Thanks. Dmitry. > > […] > > Thanks, > Matt > > > On Mon, Aug 17, 2015 at 1:26 PM, Matt Wilmas > > […]