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

Serializing all types to strings? #26

Open
maria-p opened this issue Jul 26, 2012 · 25 comments
Open

Serializing all types to strings? #26

maria-p opened this issue Jul 26, 2012 · 25 comments

Comments

@maria-p
Copy link

maria-p commented Jul 26, 2012

Hello,

I'm trying to use your bundle for our application REST API, and I'm serializing my objects to json format.

I need to keep parameter types when serializing, so that null values will be nulls after serialization, and integers remain integers. But I found that for some reason you convert all the json values to strings when calling getClientData() method in serializeForm(). If I change this to getNormData() - everything works just fine.

The question is - was that done intentionally? If yes, then why?

@beberlei
Copy link
Collaborator

It was not done intentionally, but using getNormData() breaks the serialization for everything that is not a scalar type. This will be a tricky to come up with good solution.

@beberlei
Copy link
Collaborator

@bschussek same here, i need an idea. How do i solve this problem? Its only relevant in JSON, since xml needs everything as string as well anyways.

@webmozart
Copy link

I'll check whether string casting can be deferred to buildView(). Then you'd have whatever-your-data-type-is in getViewData() and the string version of that in $view->vars['value'].

@beberlei
Copy link
Collaborator

@bschussek does whatever your data type include DateTime instances? or is that the array already?

@webmozart
Copy link

That's an array already, otherwise it couldn't be mapped to its fields.

@beberlei
Copy link
Collaborator

That would be really awesome, exactly what would be necessary here :-)

@beberlei
Copy link
Collaborator

@bschussek i dont think it makes sense to think about this. Just checked performance, its MUCH to slow.

See 714b61e#diff-4

Serializing this object graph, takes the following times on my machine:

  • FormSerializer: 0.78
  • JMS: 0.04

So it would actually make much more sense to use JMS Serializer for serializing, and FormSerializer for deserializing using a form when necessary.

@webmozart
Copy link

@beberlei I'd be interested though if you could analyze why exactly it is so much slower. I'm not surprised by the numbers, because the Form component needs to do a lot of stuff that probably isn't needed for a pure serializer. But if the results help us to improve the Form code, that'd be excellent.

@mvrhov
Copy link

mvrhov commented Jul 30, 2012

@maria-plyasunova: I'm not going to say that this is not the problem.
However I'm going to guess you are running on 64bit php just because of integers? Or are you integers never going over 2^31 -1

@beberlei
Copy link
Collaborator

@mvrhov no its really converting everything into strings :-) Thats what getViewData() does.

@beberlei
Copy link
Collaborator

@bschussek i split up the performance test into 3 parts, and you can probably guess whats the slowest part:

  • create: 0.7819
  • serialize: 0.0126
  • encoding: 0.0115
  • total: 0.8274
  • jms: 0.0429

@stof
Copy link

stof commented Jul 30, 2012

yeah, the main difference here is that JMSSerializerBundle can cache its metadata whereas the form needs to be built for each request I guess.
but I also see a case where a form can achieve something impossible for JMSSB (at least as far as I know the bundle): serializing an object with a self-referencing collection, without serializing the collection for the children.

@beberlei
Copy link
Collaborator

@stof my performance test even includes the metadata parsing for JMS.

@maria-p
Copy link
Author

maria-p commented Jul 30, 2012

@beberlei what do you mean by 'breaks the serialization for everything that is not a scalar type'? I've serialized objects while using getNormData() and it worked fine.

@stof
Copy link

stof commented Jul 30, 2012

@maria-plyasunova a datetime type has a DateTime object as norm data. And an entity type will have the entity as norm data, not the choice value.

@beberlei
Copy link
Collaborator

I opened GH-28 with a solution to integrating JMS Serializer and Form Types. It works for the use-cases of Form Serializer Bundle TestSuite, but forms are much more powerful and a bunch of use-cases can probably not be integrated easily into JMS Metadata. See the list of WIP items.

@beberlei
Copy link
Collaborator

ok, i cut performance hit by 50% by not using $form->setData() but $builder->setData() in the case of list serialization. Thats still 1000% more than JMS Serializer though.

@beberlei
Copy link
Collaborator

The JMS Form Metadata Driver in contrast is much faster, i guess the problem is that with a collection type and 20 items, building the same subform 20 times is just a big waste.

@beberlei
Copy link
Collaborator

@bschussek Actually the performance problem can easily be seperated more:

If you serialize 20 entities in a list, then the "collection form" that holds the entity type 20 times needs 50% of the time in "ResizeFormListener#onPreSetData". See here https://github.com/simplethings/SimpleThingsFormSerializerBundle/blob/master/Serializer/FormSerializer.php#L60

@webmozart
Copy link

@beberlei: Thanks for investigating. I already noticed during my performance tests that collection forms have quite a big impact on performance. I will try to find a solution for that in 2.2.

The problem is that, in theory, it would be sufficient to clone each form and only adapt whatever depends on the names (because the names are the only things that differ between the rows). The problem is that we cannot decide in advance what it is that depends on the names, so the safest solution is to rebuild the form for each row. I don't know yet how to solve this.

@webmozart
Copy link

i cut performance hit by 50% by not using $form->setData() but $builder->setData()

This is interesting. How many times did you call $form->setData() before?

@beberlei
Copy link
Collaborator

@bschussek i changed this 3176587

@webmozart
Copy link

Weird.. I guess I need to take a closer look at your serializer in the next days. It's funny that this change causes a performance increase of that scale.

@maria-p
Copy link
Author

maria-p commented Aug 23, 2012

@beberlei any updates on this?

@beberlei
Copy link
Collaborator

sadly now, i only realized that quite late and its pretty much constraint by the form architecture here. In XML this problem does not exist. @bschussek any new way of going around it?

Otherwise we could integrate a workaround that autocasts values during JSON serialization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants