Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Pieter12345 opened this issue Aug 24, 2020 · 1 comment
Open

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

Pieter12345 opened this issue Aug 24, 2020 · 1 comment
Labels
bug Things that don't work as designed

Comments

@Pieter12345
Copy link
Contributor

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).

@Pieter12345 Pieter12345 added the bug Things that don't work as designed label Dec 2, 2022
@PseudoKnight
Copy link
Contributor

#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Things that don't work as designed
Projects
None yet
Development

No branches or pull requests

2 participants