Optimize the StringUtil::deserialize() method

[leofeyer]

This PR uses a simple regex to filter strings which are definitely not serialized strings. It noticeably reduces the number of unserialize() calls and speeds up the method by up to 70% (see https://3v4l.org/F3gdM and https://3v4l.org/6oSrr).

[leofeyer]

Do we want to backport this to Contao 4.4 or even 3.5?

[Aybee]

IMHO 4.4 yes. 3.5 not necessary but would be nice, why not?

[leofeyer]

I have implemented the strncmp() check in abcd5be96a90313b308d3ec615d947e34ebdc31c.

[Toflar]

BTW: I'm against backporting this. It's a core method used in thousands of places in Contao. It's not a wise decision to bring that to older versions. It's not a bugfix (if performance was horrible, I'd consider it a bugfix but nobody complained so far) so it should go into 4.6.

[aschempp]

I am very much not sure about the content always being an array...

[Toflar]

Yeah, it would imply a BC break. Until now this worked:

$v = serialize(true);
$v = deserialize($v);

It won't be anymore with this change.

[leofeyer]

I have thought about the BC break as well. I guess I have to revert the last commit?

[ausi]

@Toflar your Code does also not work in the current version for me. I dont see a BC Break here

[Toflar]

Ah, I missed line https://github.com/leofeyer/core-bundle/blob/abcd5be96a90313b308d3ec615d947e34ebdc31c/src/Resources/contao/library/Contao/StringUtil.php#L1068. So yes, it can only handle arrays now. PR is good to merge.

[aschempp]

Wow, didn't know about that at all. LGTM then.

[leofeyer]

I still think that there is a BC break:

$v = serialize(true);
$v = deserialize($v);

In Contao 4.4 $v would be array(true) and in Contao 4.6 it would be b:1;, wouldn't it?

[ausi]

No, it would not AFAIK. Did you test this?

[leofeyer]

I have now and you are right. There is no BC break. 🙄