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

Exceptions in new version #41

Closed
promontis opened this issue Oct 24, 2019 · 10 comments
Closed

Exceptions in new version #41

promontis opened this issue Oct 24, 2019 · 10 comments
Labels
performance Need for speed question Further information is requested

Comments

@promontis
Copy link
Contributor

Hi @dstelljes!

We see a big performance degrade in creating the schema's when switches to master (I think this is 3.0). I think this is because of the many exceptions being thrown in code like:

   foreach (var @case in Cases)
            {
                try
                {
                    return @case.Read(element, cache, scope);
                }
                catch (UnknownSchemaException exception)
                {
                    exceptions.Add(exception);
                }
            }

            throw new AggregateException($"No schema reader case matched {element.ToString()}", exceptions);

Such code is located in JsonSchemaReader, TypeResolver, BinarySerializerBuilder.

This is especially performance heavy when the type/schema is located at the end of the array.

For this POCO...

	public class Foo
	{
		public string Bar { get; set; }
		public DateTime Date { get; set; }
	}

... I'm currently getting 391 exceptions. All handled, but it takes a long time.

@dstelljes
Copy link
Member

The goal of collecting all of those exceptions is to make debugging easier. Previously, it was sometimes difficult to tell exactly why something was failing to build.

Does your use case require that you generate many serializers/deserializers? There is a performance hit, but only when the delegate is generated—there shouldn’t be any changes in speed of the generated delegates, which is what we’re more concerned about.

@promontis
Copy link
Contributor Author

promontis commented Oct 24, 2019

The use case is indeed many (de)serializers. We first thought that was something with the Unsafe nuget package locking up, but now I've created a small sample app and it takes a relatively long time:

Start CreateProducerForType
Stop CreateProducerForType
CreateProducerForType took: 00:00:32.7368636
Start CreateNullProducer
Stop CreateNullProducer
CreateNullProducer took: 00:00:07.6819759

The sample is here: https://gist.github.com/promontis/648a9f337706819767c5df7861fac0bc

So basically it takes 32 secs to just create the Foo schema, and 7 secs for the null schema.
Is this something you are experiencing as well?

@promontis
Copy link
Contributor Author

That's debug mode btw... will try in release mode.

@promontis
Copy link
Contributor Author

A lot faster...

Start CreateProducerForType
Stop CreateProducerForType
CreateProducerForType took: 00:00:00.8040465
Start CreateNullProducer
Stop CreateNullProducer
CreateNullProducer took: 00:00:00.0347538

@dstelljes
Copy link
Member

dstelljes commented Oct 24, 2019

This may have to do with tracing/instrumentation. I don’t think debug is inherently that slow; the Chr.Avro.Binary unit tests executing in debug build several hundred delegates and still complete quickly (242 test cases, ~2 seconds).

@promontis
Copy link
Contributor Author

promontis commented Oct 24, 2019

This may have to do with tracing/instrumentation

I think so too, but I'm not using weird debug settings. Diagnostic tools are disabled. Exceptions settings never stops on exceptions.

It is still very slow/unworkable.

@dstelljes
Copy link
Member

Can you send some details about your environment so we can try to repro (target .NET versions, Visual Studio version, any plugins, etc.)?

@dstelljes dstelljes added performance Need for speed question Further information is requested labels Oct 24, 2019
@promontis
Copy link
Contributor Author

promontis commented Oct 24, 2019

Sure!

Windows 10 Enterprise
Target .NET Core 2.2
Both Visual Studio 2017/2019 (2019 is even a little bit slower) suffer from this

No non-default plugins... though these are loaded (by default):

  • Developer Analytics Tools
  • Visual Studio Snapshot Debugger
  • Visual Studio IntelliCode

If you take my gist and run it using the debugger attached, are you getting acceptable results?

@dstelljes
Copy link
Member

dstelljes commented Oct 24, 2019

Also very slow for me when referencing source (a project reference to Chr.Avro.Confluent.csproj). When referencing it normally (a package reference to the NuGet release), the debugger doesn’t pick up any of the internal exceptions and the application runs much faster.

It’d definitely be possible to implement the case pattern without relying on exceptions, but it would be a significant amount of work and a breaking change for custom cases. Since this doesn’t affect project debugging unless a developer is referencing a local clone of Chr.Avro, I’m not sure it’s worth it.

A workaround may be to use [DebuggerHidden] or one of the other System.Diagnostics attributes.

@promontis
Copy link
Contributor Author

Cool! I'll finish the PR tomorrow probably... if you can build a prerelease, we can just reference the nuget package and everything should work a lot faster!

Thanks for helping out 🤗
Closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Need for speed question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants