Re: Re: Marking error, etc. functions as cold?

Hi Matt, On Wed, Aug 19, 2015 at 4:05 PM, Matt Wilmas <php_lists@realplain.com> wrote: > 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 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 <php_lists@realplain.com> >> 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 >>> >> >