-
Notifications
You must be signed in to change notification settings - Fork 805
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
perf(orca): reuse ObjectMapper instances #2937
Conversation
creating a new ObjectMapper instance for each Stage or other per-request contexts can be wasteful; ObjectMapper is safe to reuse across threads (after it is configured). See spinnaker/spinnaker#4367 for more details.
fa452f0
to
8e728ea
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems reasonable to me, but would like @robzienert or @robfletcher to take a look as I know that there have been some subtle bugs with objectMapper
in the past.
Are there any places where the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be fine. 99... 95... 96.7% sure.
@robzienert I did find a few places in the Orca codebase where the new ObjectMapper instance returned by Lines 41 to 45 in cd6b6ca
orca/orca-keel/src/main/kotlin/com/netflix/spinnaker/orca/config/KeelConfiguration.kt Lines 62 to 66 in cd6b6ca
orca/orca-kayenta/src/main/kotlin/com/netflix/spinnaker/orca/kayenta/config/KayentaConfiguration.kt Lines 67 to 69 in 2b9de27
I left all of those unchanged in this PR, since from what I could tell, those call sites were not creating one ObjectMapper per request / unit of work etc. (and plus they seem to have legitimate reasons to configure the ObjectMapper instance differently than other regions of code). |
creating a new ObjectMapper instance for each Stage or other per-request contexts can be wasteful; ObjectMapper is safe to reuse across threads (after it is configured). See spinnaker/spinnaker#4367 for more details.
creating a new ObjectMapper instance for each Stage or other per-request
contexts can be wasteful; ObjectMapper is safe to reuse across threads
(after it is configured). See spinnaker/spinnaker#4367 for more details.