-
Notifications
You must be signed in to change notification settings - Fork 317
Perf | Use source generation to serialize user agent JSON #3772
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
base: main
Are you sure you want to change the base?
Perf | Use source generation to serialize user agent JSON #3772
Conversation
|
Changes look good, and we should merge this PR even though the design has changed back to pipe-delimited and we won't be using JSON for the User Agent. Still worthwhile to have this in the commit history so we can refer to it in the future if necessary. |
|
/azp run |
|
Azure Pipelines successfully started running 4 pipeline(s). |
|
@edwardneal This will need a rebase/merge from main to pickup the new PR pipeline configs. |
|
Thanks @paulmedynski, I've just merged and pushed. |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
This pull request has been marked as stale due to inactivity for more than 30 days. If you would like to keep this pull request open, please provide an update or respond to any comments. Otherwise, it will be closed automatically in 7 days. |
|
@edwardneal Can you rebase with main? I'll kickstart a fresh pipeline run. |
b8c33fb to
3cadb90
Compare
|
Thanks @apoorvdeshmukh, I've just rebased and force-pushed. With that said, I can see #3826 now, and I've got no objection if we want to skip this PR in favour of that one. |
Description
#3582 added functionality to send a small user agent JSON structure to SQL Server. While this works, it also drags in a reflection-based dependency, and the populated reflection-based information is held in memory, preventing the AssemblyLoadContext from being unloaded.
This PR swaps this over to using the built-in source-generated JSON serializer. This should improve performance slightly, and it's the second of a set of changes needed to enable AssemblyLoadContext unloading.
While I don't think this'll be a problem, it's worth noting that this means we'll need to compile the netfx library using the net6.0+ SDK.
Issues
Builds on #3582. Contributes to #1687.
Testing
Automated unit tests continue to function.