Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:109848
MIME-Version: 1.0
References: <DM6PR07MB661819D3D9D654287317A56DF9D10@DM6PR07MB6618.namprd07.prod.outlook.com>
In-Reply-To: <DM6PR07MB661819D3D9D654287317A56DF9D10@DM6PR07MB6618.namprd07.prod.outlook.com>
Date: Sat, 25 Apr 2020 18:53:28 +0200
Message-ID: <CAF+90c_gQjxhsq8wXymXcRBT_FY=p0B-+ufmxz4tJ-9JfFpxcw@mail.gmail.com>
To: tyson andre <tysonandre775@hotmail.com>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000021810405a420530f"
Subject: Re: [PHP-DEV] Optimizing assembly for spl_object_id(), fdiv(), etc in
 the JIT?
From: nikita.ppv@gmail.com (Nikita Popov)

--00000000000021810405a420530f
Content-Type: text/plain; charset="UTF-8"

On Sat, Apr 25, 2020 at 6:44 PM tyson andre <tysonandre775@hotmail.com>
wrote:

> Hi internals,
>
> I was wondering if the maintainers of the Opcache Jit would be open to
> work on optimizing functions such as intdiv(), fdiv(), and spl_object_id()
> in the JIT to assembly code when it was safe to do so.
> These functions have simple C implementations, so the performance overhead
> of calling PHP functions in the generated assembly
> would be noticeable compared to emitting optimized assembly.
>
> I'd expect that it'd be safe to do so under the following circumstances:
>
> - All arguments are Compiled Variables(CV) in the opcodes (i.e. $var)
>   (
> http://nikic.github.io/2017/04/14/PHP-7-Virtual-machine.html#variable-types
> )
> - The types and count of the function arguments are known to be strictly
> correct, e.g.
>   - Opcache infers they will not throw a TypeError or emit undefined
> variable warnings
>   - maybe strictly accept floats but not integers for fdiv
> - The return value of the expression won't throw
>   (e.g. don't perform this optimization for `$y = spl_object_id($x)` when
> a CV is the return value)
> - The function in question exists and is not disabled.
>
> For example, these operands would be seen by the JIT for this sequence of
> opcodes
>
> ```php
> /*
> 0000 CV0($x) = RECV 1
> 0001 INIT_FCALL 1 96 string("spl_object_id")
> 0002 SEND_VAR CV0($x) 1
> 0003 V2 = DO_ICALL
> 0004 T1 = INIT_ARRAY 1 (packed) CV0($x) V2
> 0005 RETURN T1
> */
> function create_set(stdClass $x) : array {
>     return [spl_object_id($x) => $x];
> }
> ```
>
> I expect it to be technically feasible to check for the sequence of
> opcodes INIT_FCALL, SEND_VAR (repeated), and DO_ICALL, in
> ext/opcache/jit/zend_jit.c. The resulting assembly would be smaller and
> much faster.
> (this would be done before emitting any assembly - skip over the opcodes
> for INIT_ARRAY and SEND_VAR and DO_ICALL)
>
> - What do the maintainers of the JIT module think of this idea?
>   (e.g. are there concerns about this making it harder to understand the
> JIT codebase, adding potential bugs, or slowing down the generation of
> assembly)
> - Are there any general guidelines (or talks/articles) you'd recommend for
> new contributors to the Opcache JIT? I couldn't find a README and
> https://wiki.php.net/rfc/jit doesn't seem to make recommendations that
> apply to my question.
>
> ```
> // core fdiv C implementation (does not throw for a divisor of 0)
>         RETURN_DOUBLE(dividend / divisor);
> // core of spl_object_id C implementation
>         RETURN_LONG((zend_long)Z_OBJ_HANDLE_P(obj));
> ```
>
> I started looking into this because I work on an application that heavily
> uses spl_object_id()
> and would see a small performance benefit from this,
> and was reminded of this when looking at the get_resource_id() proposal.
> (
> https://github.com/phan/phan/search?q=spl_object_id&unscoped_q=spl_object_id
> )
>
> Thanks,
> - Tyson
>

Hi Tyson,

Our general approach to this is to first add a VM opcode for the operation,
which will also provide a benefit if the JIT is not used. There's already
plenty of those, see ZEND_STRLEN for example. Adding JIT support for the
opcode would then be the natural second step.

There aren't any hard rules for when this should be done, but I believe
having some evidence that the operation is both common and performance
critical would be good.

Nikita

--00000000000021810405a420530f--