[PERFORMANCE] Various small enhancements detected thanks to flame graphs - #435
Merged
Conversation
Contributor
Author
Contributor
Author
Arsnael
approved these changes
May 17, 2021
rouazana
reviewed
May 17, 2021
Arsnael
approved these changes
May 17, 2021
Contributor
Author
This method is responsible of ~2% of total memory allocation as per async-profiler and the builder is the main guilty: Its advanced flags filtering capability, not needed for a copy use case come at a high cost.
By calling `messageMetadata` too frequently we generate too much flag copies that are responsible of over 2% of total memory allocation as per async-profiler.
…wice Field::getBody already perform the operation hence there is no need for it. Async-profiler indicates we spend 0.2% of the CPU needlessly that way... Minor but always good to take!
It was generating it for each requests. Each endpoints needs to initialize its own URI parser. Also the version was parsed for each routes and not just once per request.
String.format was evaluated on each Version and cost 0.26% of total CPU time budget.
…sChildren 0.56% running an inefficient algorithm to state which mailboxes have kids, in O(n2). While the percentage is low acting on it with likely improve p99 for `Mailbox/get` with many mailboxes and is thus worth writing.
Filters are dependent of client request, requiring ObjectMapper reconfiguration as object mapper configuration changes are not thread safe. However ObjectMapper javadoc states the following: ``` Method is typically used when multiple, differently configured mappers are needed. Although configuration is shared, cached serializers and deserializers are NOT shared, which means that the new instance may be re-configured before use; meaning that it behaves the same way as if an instance was constructed from scratch. ``` This sounds like our use case! This conforts to advices of this page: https://github.com/FasterXML/jackson-docs/wiki/Presentation:-Jackson-Performance ``` Reuse heavy-weight objects: ObjectMapper (data-binding) and JsonFactory (streaming API) ```
chibenwa
force-pushed
the
flame-graph-opts
branch
from
May 18, 2021 05:47
c4a5602 to
b64aeb0
Compare
Arsnael
approved these changes
May 18, 2021
jeantil
approved these changes
May 18, 2021
This file contains hidden or 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
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.



Here we can see a call to
MessageResultImpl::getMessageIdresponsible of0.78%of the total allocated memory... Rationals are that and intermediate (uneeded) on the fly messageMetadata is used, which includes an (innefficient) flag copy.Here is a flame graph capture showing the costs of initializing JMAP routes for each requests.