Skip to content

Optimize XContentParserUtils.ensureExpectedToken #62691

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

Conversation

original-brownbear
Copy link
Member

Wanted to clean up some deserializers using this method and noticed that it doesn't inline very well, since we use this in a number of very hot spots I think this is a worthwhile change:

We only ever use this with XContentParser no need to make it inline
worse by forcing the lambda and hence dynamic call-site here.
=> Extracted the exception formatting code path that is likely very cold
to a separate method and removed the lambda usage in hot loops by simplifying
the signature here.

We only ever use this with `XContentParser` no need to make it inline
worse by forcing the lambda and hence dynamic callsite here.
=> Extraced the exception formatting code path that is likely very cold
to a separate method and removed the lambda usage in hot loops by simplifying
the signature here.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/Core)

@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Sep 21, 2020
Copy link
Contributor

@romseygeek romseygeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice cleanup.

@original-brownbear
Copy link
Member Author

Thanks @romseygeek !

@original-brownbear original-brownbear merged commit f34246b into elastic:master Sep 21, 2020
@original-brownbear original-brownbear deleted the faster-expected-token-check branch September 21, 2020 15:03
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Oct 5, 2020
We only ever use this with `XContentParser` no need to make it inline
worse by forcing the lambda and hence dynamic callsite here.
=> Extraced the exception formatting code path that is likely very cold
to a separate method and removed the lambda usage in hot loops by simplifying
the signature here.
original-brownbear added a commit that referenced this pull request Oct 5, 2020
We only ever use this with `XContentParser` no need to make it inline
worse by forcing the lambda and hence dynamic callsite here.
=> Extraced the exception formatting code path that is likely very cold
to a separate method and removed the lambda usage in hot loops by simplifying
the signature here.
@original-brownbear original-brownbear restored the faster-expected-token-check branch December 6, 2020 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label >non-issue Team:Core/Infra Meta label for core/infra team v7.10.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants