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

Performace issue #375

Closed
aspektxxx opened this issue Mar 11, 2020 · 18 comments
Closed

Performace issue #375

aspektxxx opened this issue Mar 11, 2020 · 18 comments
Assignees
Labels
bug (cc: fix) Something isn't working

Comments

@aspektxxx
Copy link

Hi @Mike-EEE, I'm experiencing a performance issue when trying to serialize pretty complex object with few dozens of items with custom serializer in list. Currently I'm unable to pinpoint it to the root cause and prepare the repro sample. However, profiler show a 25% time is spend in SetterFactory:39 lambda.Compile();

the serialize is created with

.EnableAllConstructors()
.EnableParameterizedContentWithPropertyAssignments()
.Extend(WritableParameterizedContentExtension.Default)
.EnableImplicitTyping(allTypes.Where(x => !x.CustomAttributes.Any(a => a.AttributeType == typeof(CompilerGeneratedAttribute))))
.UseOptimizedNamespaces()

Any idea what could be root cause?

@Mike-E-angelo
Copy link
Member

Wow yes @aspektxxx lambda compile time is expensive and it should be caching this. Are you able to reduce the possible culprits by removing some of the configurations above? Or are you saying the above configurations for sure cause the performance pain you are experiencing?

@Mike-E-angelo
Copy link
Member

Also I am glad to hear you are using a complex scenario and it isn't exploding at first sight. 😁😆

@aspektxxx
Copy link
Author

@Mike-EEE well, that's the problem, I'm unable to reduce it as its too complex. Till now I was unable to prepare repro steps as all simpler examples works like a charm. I was hoping that you might have some clue what can be possible root cause. I will look more thoroughly into it next week and try to prepare repro.

@jhranac
Copy link

jhranac commented Mar 17, 2020

Facing the same (very similar) issue.

We've got a list of very complex objects. There are custom serializers defined for their types - so I'd expect the XML serializer to not care about the contents of the objects. But even with the custom serializers configured the XML serializer is scanning everything.

Based on the stack-trace there was no caching on the way that could prevent this. While adding caching to the SetterFactory directly fixes some of the performance issues (about 50% boost for me), the objects are so complex that it is insufficient.

From my side there are two issues:

  1. Missing caching (either in the SetterFactory or somewhere higher - ObjectTypeWalker?)
  2. Serializer knows about inner structure of objects for which a custom serializer is provided (I do not know if this is something that can be removed safely)

As for now we're using a workaround - in the serializer configuration we've set all members of the complex types to be ignored.

Performance before (on the left) and after caching was added to the SetterFactory (on the right)
Performance_xmlserialization

@Mike-E-angelo
Copy link
Member

Mike-E-angelo commented Mar 17, 2020

OK thank you for the additional information @jhranac. Just so we remove one element here... I would like to verify how the serializer is being created and how the serializer is being used.

The serializer should be a singleton (or scoped) object that is created once (where all the setup and caching occurs), and then its Serialize/Deserialize methods called many times throughout the lifetime of the application (with the first being the be most intensive and then subsequent calls partake in caching which should be much less).

Can you confirm that you are following this approach? That is, the serializer isn't being created at the time of the serialization and has been created beforehand (preferably during application initialization)?

@jhranac
Copy link

jhranac commented Mar 17, 2020

We're using 3 instances of the serializer in the whole application (they have slightly different configuration based on usage). All of them are singletons and live for the whole lifetime of the application, never to be disposed/re-created. They do not cross-reference each other or use custom serializers to mix the usage. Always using just the one for the job.

@Mike-E-angelo
Copy link
Member

ALRIGHT... crossing that off my list here. The next checkdown I spy is the use of UseOptimizedNamespaces() ... there might be something funny/unexpected there that we are not accounting for in the caching. I will see if I can carve some time out for this.

Are you able to perhaps try a quick scenario without the use of UseOptimizedNamespaces() and see if that impacts your overhead at all, @jhranac?

@Mike-E-angelo Mike-E-angelo added the bug (cc: fix) Something isn't working label Mar 18, 2020
@Mike-E-angelo Mike-E-angelo self-assigned this Mar 18, 2020
@Mike-E-angelo
Copy link
Member

Doh... we indeed have some funny business going on here. It looks like caching simply is not working at all. :| Looking into this now.

@Mike-E-angelo
Copy link
Member

Alright there was a place where indeed lambda compilation was occurring twice. I am not convinced this addresses the full problem but it's a start. Please find the build details here:

#381 (comment)

We'll see how that does as a first step. 🤞

@jhranac
Copy link

jhranac commented Mar 18, 2020

We're using the optimized namespaces - which in retrospect makes no sense in some of the cases so I'll remove that. That should fix some of the performance issues for us.

Performance times - 100 iterations run to get an average. Times format: [serialize 1 element in array] / [serialize 100 elements in array] in milliseconds

Version Optimized namespaces Disabled properties Optimized namespaces; disabled properties NOT optimized namespaces; properties enabled
Release version 163 / 2272 40 / 176 40 / 184 73 / 215
Test version 140 / 369 40 / 183 43 / 175 71 / 207

So the fix you've done works!
There is still some performance loss because the classes structure is scanned by the serializer even when a custom one is provided.

@Mike-E-angelo
Copy link
Member

WOW! That is some decent feedback and information @jhranac thank you for providing it. Good to see. I am getting the feeling there is another #361 that is lurking in here. Let me see if I can find it.

@Mike-E-angelo
Copy link
Member

OK @jhranac I think I got it. 😁

#381 (comment)

I removed the check for type members if a custom serializer is present. All tests still pass, so... let's see.

@jhranac
Copy link

jhranac commented Mar 19, 2020

The timings look much better now!

Version Optimized namespaces Disabled properties Optimized namespaces; disabled properties NOT optimized namespaces; properties enabled
3.1.2.3-rywdiqno 56 / 213 42 / 192 45 / 198 45 / 196

I'm happy with those results. Good job!

@Mike-E-angelo
Copy link
Member

SWEET! It's the little things in life ATM. 😅 I will merge this in and then shoot for next Tuesday for a release to NuGet. 👍

@Mike-E-angelo
Copy link
Member

Mike-E-angelo commented Mar 25, 2020

Welp, tried to deploy this but we are having a little trouble with GitHub Actions:
https://github.com/ExtendedXmlSerializer/home/runs/533449304

I have a ping out to support, we'll see what's up.

GitHub
A highly configurable and eXtensible Xml serializer for .NET. - ExtendedXmlSerializer/home

@Mike-E-angelo
Copy link
Member

Mike-E-angelo commented Mar 26, 2020

Alright, looks like the issue is with a dependency having to deal with PowerShell 7, which for some reason was auto-updated without confirmation in GitHub Action images. 🤷‍♂️

They are aware of it and looking into it:
baldator/Poshstache#10

We'll proceed once there's a fix. Sit tight with the preview feed in the meantime. 👍

@Mike-E-angelo
Copy link
Member

Also being tracked here:
actions/runner#387

@Mike-E-angelo
Copy link
Member

Mike-E-angelo commented Mar 30, 2020

FINALLY... got this sucker out:
https://www.nuget.org/packages/ExtendedXmlSerializer/

Please let me know if you run into any problems. Thank you for your efforts towards improving ExtendedXmlSerializer. 👍

An extensible Xml Serializer for .NET that builds on the functionality of the classic XmlSerializer with a powerful and robust extension model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug (cc: fix) Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants