CommandHelper

CommandHelper

46.5k Downloads

array()'s cached string representation can go out of sync

Pieter12345 opened this issue ยท 1 comments

commented

The cache used for array() its string representation runs out of sync with the actual array data in the following examples:

@a = array(1);
@b = array(@a);
@c = array(@a);
msg(@b); // Caching happens here, result is `{{1}}`.
msg(@c); // Caching happens here, result is `{{1}}`.
@a[0] = 2; // The array marks its last parent's cache as dirty, but not its other parent.
msg(@b); // Return cached result `{{1}}`, which is wrong at this point.
msg(@c); // Re-cache, returning the actual result `{{2}}`.
@a = mutable_primitive('test');
@b = array(@a);
msg(@b); // Caching happens here, result is `{test}`.
@a[] = 'newValue';
msg(@b); // Return cached result `{test}`, which is wrong at this point.

For arrays, marking all parents as dirty can be a solution (whereas it currently only marks the last parent as dirty). For mutable primitives, or user objects, the solution is not trivial. In my opinion, we could go without caching array string representations as soon as #1225 is merged and a similar solution for not sconcatting all code together is implemented in non-strict mode. I believe that arrays are not often used as their string representation, and if they are, they are rarely used twice (in which case the cache would improve performance).

commented

#1225 was merged, and when in non-strict mode it no longer sconcats when using semi-colons.

A place this might be an issue still is checking equality for arrays. It's not optimal but sometimes people do @arrayA == @arrayB. While one of the arrays would probably not benefit from a cached string, the other array might. But in general it might be nice if there was just a code path for that in CArray so that it doesn't have to get val() anyway. Just checking size() first would see a huge difference in non-equality performance.