Clone CreateResponseOptions #904
Conversation
christothes
requested review from
ShivangiReja,
joseharriaga and
jsquire
as code owners
| @@ -633,7 +633,7 @@ internal virtual CreateResponseOptions CreatePerCallOptions(CreateResponseOption | |||
| { | |||
| CreateResponseOptions clonedOptions = userProvidedOptions is null | |||
There was a problem hiding this comment.
Should we consider doing a thread-static shared instance for default options rather than creating an instance per-call?
There was a problem hiding this comment.
What scenario did you have in mind where we'd use the default options? My assumption is that we'll typically only clone when someone is passing in a non-default options instance.
There was a problem hiding this comment.
We're doing a new() if there's no custom options passed. My suggestion is to replace that with a thread-static version. We need to be able to safely mutate for this call but there's no reason why we need a fresh instance each time, is there?
There was a problem hiding this comment.
Ah, makes sense. For some reason I thought this comment was on the clone method.
| [CodeGenType("CreateResponse")] | ||
| public partial class CreateResponseOptions | ||
| { | ||
| internal static readonly CreateResponseOptions Default = new CreateResponseOptions(); |
There was a problem hiding this comment.
Will this work without being [ThreadStatic]? Won't we end up with concurrency issues because of the mutation during operations?
2 participants
No description provided.