Update AV2230: Only use the dynamic keyword when talking to a dynamic object#380
Update AV2230: Only use the dynamic keyword when talking to a dynamic object#380dennisdoomen wants to merge 1 commit into
dynamic keyword when talking to a dynamic object#380Conversation
Split from #298. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| The `dynamic` keyword has been introduced for interop with languages where properties and methods can appear and disappear at runtime. Using it can introduce a serious performance bottleneck, because various compile-time checks (such as overload resolution) need to happen at runtime, again and again on each invocation. You'll get better performance using cached reflection lookups, `Activator.CreateInstance()` or pre-compiled expressions (see [here](https://andrewlock.net/benchmarking-4-reflection-methods-for-calling-a-constructor-in-dotnet/) for examples and benchmark results). | ||
|
|
||
| While using `dynamic` may improve code readability, try to avoid it in library code (especially in hot code paths). However, keep things in perspective: we're talking microseconds here, so perhaps you'll gain more by optimizing your SQL statements first. | ||
| The `dynamic` keyword has been introduced for interop with languages where properties and methods can appear and disappear at runtime. Using it can introduce a serious performance bottleneck, because various compile-time checks (such as overload resolution) need to happen at runtime. Use it sparingly, and only when you have no other choice. |
There was a problem hiding this comment.
The original recommendations still hold; see Stephen Toub's performance series. The last sentence may lead a beginner to think there is no other choice, which is almost never true.
I think the changes to this rule should be reverted.
There was a problem hiding this comment.
I don't get your point. Isn't the new version just a shorter version?
There was a problem hiding this comment.
My point is that the original version contained important information that was removed:
- "again and again on each invocation": It's not just a one-time startup cost that users may be unaware of.
- The original version provided guidance on better alternatives (though slightly outdated; a cached
Delegate.CreateDelegatecan also be used if all types used in the signature are known at compile-time, as described in Stephen's post) - In the new version, it's unclear when someone has no other choice (combined with a lack of alternatives)
- The removed bullet that puts the perf impact into perspective was removed. Code that rarely executes or is part of a dev-local or CI setup may benefit from cleaner code rather than reflection.
There was a problem hiding this comment.
Alright. I don't care about this rule as I never use dynamic, but given your arguments, I'll keep it as-is.
There was a problem hiding this comment.
Thanks.
Unfortunately, I have a case where dynamic is heavily used, because it involves generic type substitution. I'd like to get rid of it, but investigating the alternatives takes time and I never get to it. Once I know more, I can probably propose up-to-date guidance for this rule.
There was a problem hiding this comment.
Ask your friendly neighborhood AI agent to do it ;-)
2 participants
This PR updates guideline AV2230.
It was split out of #298 so the change can be reviewed independently.
Files:
Part of the replacement for #298.