@withinboredom I've only skimmed this PR, and interning types seems like an interesting idea. Would it be possible to achieve the same thing by interning complex types, rather than creating a new data structure? This would not require changes in the type layout, but just make complex types point to the interened storage (since they are already pointers). The fast path can then use direct comparison of the complex type pointers, and use the existing type comparison as a fallback.

🤦 of course @iluuu1994! That's probably far simpler, to be honest. I was experimenting with the following logic and needed something easier to fiddle with:

Given a type constraint: A|(C&D)|E and a type F = (C&D&Z) (class F extends C implements D, Z).

When we check that F satisfies A|(C&D)|E, we will find that F satisfies C&D after a search.

We can then update the type to include F so that it is A|(C&D)|E|F so that we don't need to do a search for that type anymore.

When we check that F satisfies A|(C&D)|E, we will find that F satisfies C&D after a search.

This has union-find/equivalence classes vibes, might be interesting to take a look at that if you're interested in these kinds of optimizations.

Is this worth pursuing further, or should I take a different approach altogether?

This is worth pursuing IMO, at least for the type-checking performance gain.

Possibly, a runtime cache of zend_check_user_type_slow() would bring additional gains. Currently we use the cache slots for class resolution, but we could add a slot for the last positive type check. Type checks are positive most of the time.

As @iluuu1994 said you can probably get away with only interning zend_type_list.

You will want to support Opcache: Types from cached scripts should be stored in SHM, and when a cached script is loaded, its types should be added to the tree. Two scripts may independently declare the same type (in two different requests), and then loaded in the same request, but maybe they can be de-duplicated during script persistence.

Regarding memory management: You can probably use the arena by default. IIRC all types are allocated on the arena currently (and moved to SHM during persistence). This simplifies memory management as you don't need to free types, and you don't need to know if a type is in SHM.

This is something I wanted to try in the context of generic arrays, but didn't get to it. I'm glad you are working on it. (The use-case was that with reified generics we needed to create zend_types at runtime, often by unioning two types. Interned types would allow to have a cache of union operations, to make the operation less expensive and use less memory.)

I think @Girgias also had the idea of interned types in the context of type aliases.

I've finished constructing a PoC using zend_type and only zend_type, it is much more different than this, but is actually O(1) type checking, when using opcache to intern the types (resulting in a ~2-5% speedup for basic type checks). When not using opcache, it still appears to speed things up, but not that much. My only concern about the new implementation is that it appears to cause a lot more page faults than the existing implementation. I think I can resolve that by learning more about how oparray works to make sure the required memory is nearby during runtime.

I should have a PR in another week or two -- I'll be traveling for work for this week and won't have as much time to finish cleaning up my implementation.

How it works:

Basically there is a HashTable of HashTables. To write it in php itself, it would look something like this:

// represent a type that could be A|B, A&B or, A>B
$constraintCache = [];
$constraintCache[canonical_hash($zend_type_list)] = [
 ce_hash($class_a) => true, 
 ce_hash($class_b) => true,
];
// find if the constraint is satisfied with a given type
$satisfied = $constrainCache[canonical_hash($zend_type_list)][ce_hash($class_a)] ?? false;

Since we are using interned types, the "hash" is literally the pointer address of the zend_type_list and/or zend_class_entry. The only issue I think it may cause is that it needs to reorder type signatures when interning them (so that A|B == B|A) which will be visible to users during error messages. I don't know if that will be an issue.

As Arnaud said I was having a think about this, but mainly in the context of compile time resolution for self/parent at compile. The current issue I have is with traits as changing the arginfo requires a deep copy of a function, which is not reasonable.

Being able to do a pointer equality check for a type would definitely speed up the basic case of 2 types being equal which would be useful for variance checking.

I can see the benefit for this as well for type checking more complicated types like a function type. But I don't really see how this helps for type checking that an object satisfies an intersection or union type? Mainly because I don't see how a tree would cover all cases and complicated unions/intersections.

Since we are using interned types, the "hash" is literally the pointer address of the zend_type_list and/or zend_class_entry. The only issue I think it may cause is that it needs to reorder type signatures when interning them (so that A|B == B|A) which will be visible to users during error messages. I don't know if that will be an issue.

@withinboredom this is already the case for built-in types, so no this is not an issue from our PoV. (See https://3v4l.org/qqP8E)

Possibly, a runtime cache of zend_check_user_type_slow() would bring additional gains. Currently we use the cache slots for class resolution, but we could add a slot for the last positive type check. Type checks are positive most of the time.

@arnaud-lb @iluuu1994
As this was mentioned in my PR, I've PoC'd this here (on own repo as CI on php org broke): nielsdos#123 ; Just to let you know. According to benchmark CI makes slight degradation on Symfony and slight improvement on Wordpress.

@nielsdos It might be faster to inline the cache check into zend_verify_recv_arg_type_helper(), but not sure.

@nielsdos It might be faster to inline the cache check into zend_verify_recv_arg_type_helper(), but not sure.

Yes-ish. Then we need to check the type to be object earlier, which might be better or worse in some cases. I'll try it out and check how it impacts phpstan. On phpstan when analyzing phpstan itself zend_verify_recv_arg_type_helper takes 16% of runtime due to a cache miss on the arginfo type info. But let's continue the talk in my PR.