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

very poooor performance when debugger attached #669

Open
springy76 opened this Issue Mar 27, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@springy76

springy76 commented Mar 27, 2018

I'm using NJSonSchema indirectly by using NSwag.

If there is no debugger (=VS2017 15.6) attached then generating swagger.json takes 2-3 seconds (still way too slow, Swashbuckle takes a fraction of a second for the very same app).

If the debugger is attached then the same thing takes 4-5 MINUTES !!

It took my some time to notice that the event log of Diagnostic Tools was flooded with Threading: Task "<unknown Id>" Started. messages, some thousand per second (since timestamp was only changing on second fraction every 10-20 messages).
Historical debugging always shows FileExistsAsync in stack trace, which always calls Task.Factory.StartNew (IMHO a very bad thing).

Since the only caller of FileExistsAsync is this position I would suggest putting an empty document into the cache instead of the value null so it only happens once per file.

@RSuter

This comment has been minimized.

Owner

RSuter commented Mar 27, 2018

I know there is still a lot of potential to improve performance - right now some things are not really optimized for speed. I think most of the time is used for $ref resolution (JsonPathUtilities) because it has to scan the whole graph (even multiple times). But yes, FileExistsAsync and all these file APIs should not use Task.Run but directly call the sync methods...

Can you create a PR?

@RSuter RSuter added the enhancement label Mar 27, 2018

@RSuter

This comment has been minimized.

Owner

RSuter commented Mar 27, 2018

Probably we should remove all these Task.Factory.StartNew in DynamicApis

@springy76

This comment has been minimized.

springy76 commented Mar 27, 2018

I'm currently just using NuGet packages but from the source here it's very obvious that Dictionary<string, XDocument> Cache does not handle Exists = false results.

@RSuter

This comment has been minimized.

Owner

RSuter commented Mar 27, 2018

RSuter added a commit that referenced this issue Mar 27, 2018

@RSuter

This comment has been minimized.

Owner

RSuter commented Mar 27, 2018

The most expensive part is the $ref resolution:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment