Skip to content
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

Performance issues with i18n of HAL link titles #1019

Closed
slivotov opened this issue Jul 15, 2019 · 6 comments
Closed

Performance issues with i18n of HAL link titles #1019

slivotov opened this issue Jul 15, 2019 · 6 comments
Assignees
Milestone

Comments

@slivotov
Copy link

slivotov commented Jul 15, 2019

In case of creating link without Title there is no way how to instruct HATEOAS not to try to resolve locale Title.

In our case, most of the links do not require title publishing, so we are not adding such field in Link object. However, when HalLinkListSerializer is performing serialization, internally during converting Link object into HalLink object it is trying to resolve link title name always

private HalLink toHalLink(Link link) {

	HalLinkRelation rel = HalLinkRelation.of(link.getRel());

	return new HalLink(link, getTitle(rel));
}

Internally NoSuchMessageException is thrown, which is creating a hard pressure on our CPUs.
The below profile was created using async-profiler which is visualizing profile with FlameGraph. As you can see we are spending a lot of time in the native code of filling stack trace.
Screenshot 2019-07-15 at 10 21 12 PM

Probably I'm missing some bigger picture, but is it possible to fix this issue? Or at least work around it somehow(I tried but hadn't found an easy way).

@odrotbohm odrotbohm self-assigned this Jul 15, 2019
@odrotbohm odrotbohm added this to the 1.0 RC1 milestone Jul 15, 2019
odrotbohm added a commit that referenced this issue Jul 15, 2019
HalLinkRelation now implements MessageSourceResolvable.getDefaultMessage() returning an empty String to avoid NoSuchMessageExceptions for every resolution not backed by an actual translation. Tweaked serialization configuration for HalLink to not render empty title strings.
@odrotbohm
Copy link
Member

I've just pushed a fix that should avoid the exceptions being created in the first place. Snapshots should be available in a bit. Would you mind giving them a try?

@odrotbohm odrotbohm changed the title No title link high CPU utilization Performance issues with i18n of HAL link titles Jul 15, 2019
@slivotov
Copy link
Author

I'll definitely give them a try tomorrow. Need to go to prod soon ) Will you send me a version or I'll have to build it?

And thank you very much for so quick answer!

@odrotbohm
Copy link
Member

If you declare the snapshot repository (https://repo.spring.io/libs-snapshot), you can just tweak the spring-hateoas.version property in your Spring Boot application to 1.0.0.BUILD-SNAPSHOT.

@slivotov
Copy link
Author

I had tested this fix. It is working. Thank you!

@odrotbohm
Copy link
Member

Great to hear. Happy coding! 🙃

odrotbohm added a commit that referenced this issue Jul 18, 2019
Introduced MessageResolver interface and moved all components previously referring to MessageSourceResolvable to it. This allows us to directly shortcut message resolution by providing a no-op instance in case our default resource bundle rest-messages does not exist and thus avoid the overhead of attempted message resolutions in that case.
@odrotbohm
Copy link
Member

I just went a bit further to optimize for the case that a rest-messages resource bundle does not exist in the first place. We now use a dedicated no-op implementation that directly shortcuts the message resolution to avoid even more overhead of an attempted resolution which would result in no result by definition.

odrotbohm added a commit that referenced this issue Jul 31, 2019
We now properly look for resource bundles by scanning for a resource bundle *pattern* instead of a plain file named rest-messages. Added integration tests to make sure that resource bundles are not skipped even if they existed.

Added the ability to define common messages in a rest-default-messages.properties, as Spring Data REST requires that and it would be too cumbersome for Spring Data REST to additionally deploy them in its own configuration.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants