Skip to content

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Jan 20, 2024

This attempts to align the behaviour of $data as if we had a ZPP check for it, as such passing null is deprecated and non stringable objects throw an Error.

Another addition is to check that all entries of an array are actually strings and pre-concatenating them to save on IO calls (and actually do what the docs say passing an array does).

@nielsdos
Copy link
Member

Come to think of it, the array case is perhaps a good opportunity to save on system calls using the vector write API from Linux: https://linux.die.net/man/3/writev
It would require checking if it's castable to a file descriptor and doing some sanity checks though, so that's not for this PR but could be future work.

}
if (ZSTR_LEN(entry_str)) {
// Append to smart string
smart_str_append(&smart_str, entry_str);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So previously we wrote it directly to the streams and now smart_str is used. It seems to me that this can result in more to be used at a single point which might not be ideal especially if you have got lots of items in the array. This seems like a potential regression to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants