Some code in that function breaks that I don't yet fully understand. I'll need to take a closer look tomorrow.

I'm aware the fix is incomplete, but I wanted to wait for feedback first. For tracing JIT, in case of compile-time unknown property infos, we use the property info discovered during tracing. This avoids the additional check, as we set instanceof to 0. We could simplify this check further by checking for the class instead, but accessing properties from subclasses is common, so this would result in more slow path executions.

Can you clarify on your guard suggestion? I don't understand the difference to the check in this PR.

I'm aware the fix is incomplete, but I wanted to wait for feedback first. For tracing JIT, in case of compile-time unknown property infos, we use the property info discovered during tracing. This avoids the additional check, as we set instanceof to 0.

The most usual case (accessing property of $this) didn't require class guard. The code from micro_bench.php.

class Foo {
    public $b = 0;
    function read_prop($n) {
        for ($i = 0; $i < $n; ++$i) {
            $x = $this->b;
        }
    }
}

The fix adds check for property info on each loop iteration. Now take a look into something like PhpParser\\ParserAbstract::doParse() that performs hundreds properties accesses in a loop. A single class guard at start of the trace, may replace all the separate property guards.

We could simplify this check further by checking for the class instead, but accessing properties from subclasses is common, so this would result in more slow path executions.

You are right. This solution is not ideal. The class guards failure may lead to recording and compiling identical traces for different subclasses that access property declared in a base class.

Can you clarify on your guard suggestion? I don't understand the difference to the check in this PR.

I hope, I explained this above.

What was the reason in ability to add hooks in subclasses for a regular property declared in the parent?
I suppose, this shouldn't be allowed. You should be able to change the hooks, but not converting regular and "hooked" properties back and force.

A single class guard at start of the trace, may replace all the separate property guards.

I see, thank you for the explanation. Another thing we could do is add a "property hooks" guard, i.e. check that the traced object doesn't have any property hooks (ce->num_hooked_props == 0). This would lead to bigger slowdowns for objects with any hooked properties, as they would fall back to the VM, but it would avoid most of the slowdown for normal code for now. WDYT?

What was the reason in ability to add hooks in subclasses for a regular property declared in the parent?

It was a fundamental design decision to make plain properties completely compatible with hooks. Some languages only allow adding accessors to child classes for public $prop { get; set; } parent properties, and we wanted to avoid fragmenting the whole ecosystem into plain props and { get; set; } props that specify these empty hooks "just in case".

A single class guard at start of the trace, may replace all the separate property guards.

I see, thank you for the explanation. Another thing we could do is add a "property hooks" guard, i.e. check that the traced object doesn't have any property hooks (ce->num_hooked_props == 0). This would lead to bigger slowdowns for objects with any hooked properties, as they would fall back to the VM, but it would avoid most of the slowdown for normal code for now. WDYT?

I don't see why "unexpectedly appeared" property hooks should lead to exit to VM. They should be handled by side traces. So I don't see how "property hooks" would really help (may be I didn't understand yet).

What was the reason in ability to add hooks in subclasses for a regular property declared in the parent?

It was a fundamental design decision to make plain properties completely compatible with hooks. Some languages only allow adding accessors to child classes for public $prop { get; set; } parent properties, and we wanted to avoid fragmenting the whole ecosystem into plain props and { get; set; } props that specify these empty hooks "just in case".

This is a yet anther shoot in the leg. The languages designers made that restriction on purpose.

I don't see why "unexpectedly appeared" property hooks should lead to exit to VM. They should be handled by side traces.

There's no reason other than that I just don't understand it well, I don't know how this would be implemented.

So I don't see how "property hooks" would really help (may be I didn't understand yet).

My idea was basically to create a new type of guard that checks if the runtime object declares any new property hooks that the compile time ce did not (i.e. obj->ce->num_hooked_props != comp_time_ce->num_hooked_props). As you say, this could also be done once per trace, rather than for each property access, and would not prevent traces from being reused in other subclasses. But that's with a limited understanding, so it might not make sense.

The languages designers made that restriction on purpose.

That's an assumption. Newer languages do not differentiate between properties and fields, e.g. Swift or Kotlin. C# does it, but it's also historic, because fields were a thing before properties. It also has ABI implications there, which do not apply here.

Newer languages do not differentiate between properties and fields, e.g. Swift or Kotlin.

Are you sure they allow "conversion" of field to property, may be only overriding?

If this is not true you should take a look into Swift/Kotlin implementations (find related papers or speak to their experts) to understand how they solve the problem - "It's not possible to simple read object filed if some child class may convert it to property".

One more idea how this might be implemented in PHP. PHP always performs type check when reads a filed (this is stupid but even typed field may be unset). In case we might set type of filed (that is converted to property) to be IS_UNDEF, it would start to work out of the box. I suspect, this solution won't work out of the box, but may be you'll find something on top of it.

I will think about more solutions, but I will also need to take some time just understanding traces and guards better. I don't currently understand the difference between a side-exit and falling back to the interpreter, in other contexts I've seen those two used interchangeably.

I will think about more solutions, but I will also need to take some time just understanding traces and guards better. I don't currently understand the difference between a side-exit and falling back to the interpreter, in other contexts I've seen those two used interchangeably.

Initially any failed guard leads to transferring control to VM interpreter.
With ZEND_JIT_EXIT_TO_VM repeatable failures of the same guard will always fall-back to VM.
Without ZEND_JIT_EXIT_TO_VM at some point (after opcache.jit_hot_side_exit) we will record and compile a side trace and link the failed guard to the new side trace start, so on the next failure of the same guard we will directly jump to the new side trace.

Are you sure they allow "conversion" of field to property, may be only overriding?

Just to clarify: Yes, they do allow "virtual" properties, they don't just shadow the properties, if that's what you meant. E.g. see Kotlin here https://pl.kotl.in/iXAa7j5ID. I'm certainly not an expert in these languages but I suspect that they can access fields directly when the class hierarchy is known, thus allowing to omit a call to the accessor function. Of course, another great benefit there is that they are final by default. At least Kotlin, I don't remember Swift.

One more idea how this might be implemented in PHP. PHP always performs type check when reads a filed (this is stupid but even typed field may be unset). In case we might set type of filed (that is converted to property) to be IS_UNDEF, it would start to work out of the box. I suspect, this solution won't work out of the box, but may be you'll find something on top of it.

I have also thought of this. We could think about allocating another slot in the object, but this still won't help if the assignment in the JIT happens to the original property slot. So at least this part would have to be avoided in some way.

Are you sure they allow "conversion" of field to property, may be only overriding?

Just to clarify: Yes, they do allow "virtual" properties, they don't just shadow the properties, if that's what you meant. E.g. see Kotlin here https://pl.kotl.in/iXAa7j5ID. I'm certainly not an expert in these languages but I suspect that they can access fields directly when the class hierarchy is known, thus allowing to omit a call to the accessor function. Of course, another great benefit there is that they are final by default. At least Kotlin, I don't remember Swift.

I see. A bit better example https://pl.kotl.in/Dbkz5gvge and the generated JS

  function printProp(a) {
    print(a.x_1);
    print(a.get_y_1mhr68_k$());
    print(a.get_prop_wosl9o_k$());
    print(a.foo_26di_k$());
    print('\n');
  }

open properties are implemented through the virtual getter, final fields are read directly but can't be overridden. So open is a "syntactic sugar" for public $prop { get; set; }.

Unfortunately in PHP properties are not final by default.

For context, Swift does not require open. https://swiftfiddle.com/aie6px6xnvdifnosh56xb3kpt4 But they can likely discover the class hierarchy during compilation, I believe their smallest compilation unit is an entire module.

For context, Swift does not require open. https://swiftfiddle.com/aie6px6xnvdifnosh56xb3kpt4 But they can likely discover the class hierarchy during compilation, I believe their smallest compilation unit is an entire module.

This site doesn't work for me. Try to analyse the generated code. When it reads fields directly? When access through getters?

@dstogov I used https://godbolt.org/noscript/swift with the -emit-sil flag and the script below. It's unfortunately very unradable. It seems vars emit class_method ... #A.prop1!getter instructions, while let does a direct access with ref_element_addr. Anyway, I'm not sure this helps us too much...

import Foundation

class A {
  var prop: Int = 1
  let prop2: Int
  var prop3: Int

  init(prop2: Int, prop3: Int) {
    self.prop2 = prop2
    self.prop3 = prop3
  }
}

class B: A {
  override var prop: Int {
    get {
      return super.prop * 2
    }
    set {
      super.prop = newValue
    }
  }
}

func print1(a: A) {
  print(a.prop)
}

func print2(a: A) {
  print(a.prop2)
}

func print3(a: A) {
  print(a.prop3)
}

print1(a: A(prop2: 42, prop3: 43))
print2(a: B(prop2: 42, prop3: 43))
print3(a: B(prop2: 42, prop3: 43))

It seems like swift always reads properties/fields through virtual getters. (I tried your old example with -O -S). printProp() is compiled into the same asm even if I remove property overriding in B.

output.printProp(a: output.A) -> ():
        push    r13
        push    rbx
        push    rax
        mov     r13, rdi ; keep "a" in %r13                                
        lea     rdi, [rip + (demangling cache variable for type metadata for Swift._ContiguousArrayStorage<Any>)]
        call    __swift_instantiateConcreteTypeFromMangledName
        mov     esi, 64
        mov     edx, 7
        mov     rdi, rax
        call    swift_allocObject@PLT
        mov     rbx, rax
        mov     qword ptr [rax + 16], 1
        mov     qword ptr [rax + 24], 2
        mov     rax, qword ptr [r13] ; load VMT into %rax
        call    qword ptr [rax + 64] ; call some virtual property getter
        mov     rcx, qword ptr [rip + ($sSiN)@GOTPCREL]
        mov     qword ptr [rbx + 56], rcx
        mov     qword ptr [rbx + 32], rax
        movabs  rdx, -2233785415175766016
        mov     esi, 32
        mov     ecx, 10
        mov     rdi, rbx
        mov     r8, rdx
        call    ($ss5print_9separator10terminatoryypd_S2StF)@PLT
        mov     rdi, rbx
        add     rsp, 8
        pop     rbx
        pop     r13
        jmp     swift_release@PLT

I don't really understand this assembly, but the print2 assembly has an additional call instruction compared to print3. So, there seems to be a difference between var and let (let being the equivalent of const), as mentioned. let properties cannot have accessors, so that's why it's safe for them to skip the getter in this csae.

Oh, and making the properties fileprivate also does it, because then we know there can't be some other child class in a different compilation unit overriding the property.

I don't really understand this assembly, but the print2 assembly has an additional call instruction compared to print3

You messed print2 and print3. (print3 has an additional call)
print1 and print3 are almost equivalent (they call getters at different VMT offset), print2 reads property directly by offset (cheaper code).

Conclusion: Kotlin uses getters for "open" properties, this is not a big problem as the properties are final by default. Swift always use getters for "var", but optimizer may inline them in some cases. PHP with non-final properties has to check for hooks on each access (VM optimizes this caching HOOKED_PROPERTY_OFFSET), but efficient compilation with hard-coded property offsets becomes a problem. This may be partially improved by additional guards in tracing JIT, but not in function JIT. I would consider disabling conversion of fields into property by default, and allowing this only for fields with some attribute.