Optimize InlineFormatter #557
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Requires explicitly specifying the LangVersion as BDN doesn't like 'latest' when compiling the project to bench it
benbryant0
commented
Apr 25, 2021
| return Regex.Replace( | ||
| commandText, | ||
| regexPattern, | ||
| m => paramValuesByName[m.Value.Substring(1).ToLower()], |
There was a problem hiding this comment.
Capture groups are relatively expensive for .NET regex, and benchmarking showed that doing it this way would drop allocations by ~20%.
NickCraver
approved these changes
May 8, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates BenchmarkDotNet, adds a benchmark for
InlineFormatter.FormatSql, and optimizes that method.When trying to pin down performance issues with MiniProfiler it was reporting a step as taking >60 seconds when that step just wrapped an SQL command that it reported as taking ~1.5 seconds. The SQL was run through Dapper and had a huge
IN ()statement using a list parameter with ~14k items. After some investigation I found out that it only happened when there was an active MiniProfiler session and that it wouldn't happen if I put the values directly in the query instead of passing them as a parameter. Eventually I ended up atInlineFormatter.FormatSql.When benchmarking it I found that performance was dramatically worse when a parameter object's name property lacks a DB param prefix:
@:?. This triggers this line which scans the command text with regex to determine the correct prefix. This is relatively expensive and scales badly. Unfortunately, this seems to be the usual case for EF 6 parameters and Dapper list expansions (the latter being my case).I played around with a lot of different ways to improve this and I think what I have now is somewhat balanced between readability and performance. The average time with/without the prefixing is almost equivalent now, with allocation down 23x for the worst 10 param case. That increases as more params are used, so my 14k param case went from ~35s & 6.8 GB allocated to ~2.4s and 8.2 MB.
Before:
After: