Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:87797
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain realplain.com from 216.33.127.82 cause and error)
Message-ID: <23E5ED2D91D7459FA99EAF47549CC373@pc1>
To: "Dmitry Stogov" <dmitry@zend.com>
Cc: "PHP Internals" <internals@lists.php.net>
References: <74E46BF748B74E5388C90AE7AFE9373E@pc1> <CA+9eiLt90QpBhQbs+vrRXcdoTEkbwXbV=ANO3JvG186av1Pw9A@mail.gmail.com>
Date: Wed, 19 Aug 2015 08:05:35 -0500
MIME-Version: 1.0
Content-Type: text/plain;
	format=flowed;
	charset="Windows-1252";
	reply-type=original
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Re: Marking error, etc. functions as cold?
From: php_lists@realplain.com ("Matt Wilmas")

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 <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