Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:87808
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 209.85.213.171 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <23E5ED2D91D7459FA99EAF47549CC373@pc1>
References: <74E46BF748B74E5388C90AE7AFE9373E@pc1>
	<CA+9eiLt90QpBhQbs+vrRXcdoTEkbwXbV=ANO3JvG186av1Pw9A@mail.gmail.com>
	<23E5ED2D91D7459FA99EAF47549CC373@pc1>
Date: Wed, 19 Aug 2015 18:57:43 +0300
Message-ID: <CA+9eiLvadQpjasdrxw6AGrWb69Pcyw+TDKyvoRqgLpJU+6+Zdw@mail.gmail.com>
To: Matt Wilmas <php_lists@realplain.com>
Cc: PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=089e01184b0c55c0b2051dac1877
Subject: Re: [PHP-DEV] Re: Marking error, etc. functions as cold?
From: dmitry@zend.com (Dmitry Stogov)

--089e01184b0c55c0b2051dac1877
Content-Type: text/plain; charset=UTF-8

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
>>>
>>
>

--089e01184b0c55c0b2051dac1877--