Marking error, etc. functions as cold? View as Markdown

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