New handler for retrieving properties when object is serialized

On Fri, Jul 26, 2013 at 2:59 PM, Johannes Schlüter
johannes@schlueters.dewrote:

Such a way for overloading is bad. This is painful on each update. If
there are different things to do use different hooks. The
implementations might call the same implementation but a "purpose"
option leads to a long term maintenance hell whenever there is a
change.(a tiny bit better would be using an enum instead of int as in
some situations compilers/analyzers might give a hint ... but still a
mess)

Ok so probably better not to do that :)

But I still don't buy the argument for this hook.

Typically internal classes use some C data structure for keeping their
internal state. Often actually in a form that it can not be represented
in zval. From that observation (after a few years of looking at phpsrc,
pecl external and close-source exts) Serializable is the preferred way
to serialize these in an efficient way (converting everything in zvals,
then serialize, finally freeing temporary data is a quite expensive
way ..)

I agree that Serializable is better for performance. The reason why
I proposed this is to differentiate serialization properties from normal
properties in cases where Serializable would require lots
of coding for a small benefit (object is not serialized often)
or it is not an options (DateTime - see bellow).

So the purpose is to have a different serialized representation, so
there would be compatibility issues, similar to going to Serializable.

Yeah but that serialized representation is exactly the same as
the current representation (the serialized strings before the patch is
exactly the same as the string after the patch :)). The difference is that
get_properties returns empty HashTable now. The serialization is
backward compatible though.

I just want to find a solution for this problem. What would you do?
Would you use Serialazable even if it breaks apps after updating PHP.
Or would you still use get_properties (possibly with testing
serialize_lock) ?

Jakub

Such a way for overloading is bad. This is painful on each update. If
there are different things to do use different hooks. The
implementations might call the same implementation but a "purpose"
option leads to a long term maintenance hell whenever there is a
change.(a tiny bit better would be using an enum instead of int as in
some situations compilers/analyzers might give a hint ... but still a
mess)

This is a gross misrepresentation. The vast majority of extensions will
not care about this purpose, just like most extensions do not care about
get_debug_info or get_gc or about the purpose in read_property. If a new
purpose is added, this would not be any different than adding a new hook
as long as:

1. the extension doesn't purposefully break forward compatibility, for
   example by doing switch(purpose) { ... default: panic(); } and
2. the new hook would default to get_properties, either by explicitly
   setting the pointer to get_properties in the standard handlers array or
   by having the engine fall back to get_properties (the last one was the
   solution adopted for get_debug_info and get_gc afaik).

Yes, this would break extensions. But we never made any promises about
source BC for extensions. In fact, we actually break it for every
release, sometimes in rather subtle ways like in 5.4 (due to interned
strings, and object properties storage).

So what is being proposed is to extend our already too large API with
yet another way of doing something for edge cases for which we have a
good solution for the general case already.

Alternatively, could the problem perhaps be mitigated by making
available some utility functions for serializing native types?

--
Gustavo Lopes