[Refactor] How to avoid the need to be aware of zend_object in the internal structure code?

[rtheunissen]

I'm trying to separate the "internal" structures from all the zend stuff (zend_object, handlers, ce, etc), which has been smooth sailing so far. I came across a potential situation where this might not work as well as it sounds:

The case is with $vector->merge($iterable). We can take internal shortcuts when the $iterable is a php-ds structure, eg. another php_ds_vector_t. In order to access the internal buffer of that vector we have to follow this path: zval => zend_object => structure, ie. ((php_ds_vector_t*)(Z_OBJ(z)))->vector. The problem is that the current scope doesn't have knowledge of php_ds_vector_t, only ds_vector_t.

The only solution I can think of is to create separate functions, one for each shortcut, and only accept the structures, rather than a generic zval, except for the case where the Z_OBJ_CE is not a php-ds structure (array or other iterable). This would work fine, effectively moving the logic of "is this a vector? is this a deque?" out of the internal structure and into the callee's scope.

So instead of calling ds_vector_t *ds_vector_merge(ds_vector_t *vector, zval *values) I could use something like ds_vector_t *ds_vector_merge_vector(ds_vector_t *vector, ds_vector_t *values).

Is it possible to pass a void * along with a typedef to a function? That way the callee is responsible for extracting the structure, but the structure is responsible for the logic to merge it in. Would remove the need to have multiple x_merge_x, x_merge_y, x_merge_z patterns.

[rtheunissen]

#15 is directly related.

[rtheunissen]

Another example is $map->sort($callable), which expects a callable as function (Pair $a, Pair $b) {}, so the map has to create these pair objects out of the buckets in the hashtable. If the internals aren't aware of the objects, there is no way for the map to create the zval Pair objects.

I know the overhead of creating and destroying two objects for each comparison is terrible, but the only alternative is to only compare by key (and have the user look up using that key if they want to compare by value?), or to use arrays instead like [x1, y1], [x2, y2] or have the callable accept ($x1, $y1, $x2, $y2), or use a flag like DS_COMPARE_BY_VALUE.

I really like the two Pair current solution because it's flexible and consistent.

$map->sort(function(Pair $a, Pair $b) {
    return $a->value <=> $b->value;  // requires two objects per comparison
});

If by key only, that would be:

$map->sort(function($a, $b) use ($map) {
    return $map[$a] <=> $map[$b];  // requires two lookups per comparison
});

---

Alternatives:

• Instead of Pair, use an array of size 2.
• Use a comparison flag like DS_COMPARE_BY_VALUE after the callable.
• Have sort be by value, and ksort be by key. 💡
• Provide both keys and both values to the callable.

---

Even if we go with sort and ksort respectively, it still doesn't solve the original issue of how to create instances of Pair from within the map. Unless that requirement in itself indicates bad design.

[nikita2206]

Just a suggestion kinda out of scope, but most probably this is one of the heaviest lines in the callback-sorting methods? It might be that caching the last created pair(s) and reusing them if their refcount dropped to zero would be more efficient (I'm not suggesting a pool - that would involve too much overhead, it could be more basic: if there was pair_init(Pair *candidate) where candidate could be passed all the way down from user_compare_by_pair)

[rtheunissen]

I don't see a reason why that wouldn't work. I've opted to introduce Map::ksort, and have Map::sort work on the values only - similar to a PHP array. The need to create Pair objects doesn't exist in sort anymore, but it's still required for Map::pairs. I've opted to lift that code into the php_ scope, which I don't like very much but will do for now. See https://github.com/php-ds/extension/blob/refactor/src/php/objects/php_ds_map.c#L58

[rtheunissen]

^ Solution.