Performance improvements in link generation

[mst-avaleo]

After adding changes from #749 I've check the v1.0.0 (1.0.0.BUILD-20190402.182354-150) and the performance is 3 times slower than on v0.25.x. I also checked v1.0.0 earlier and I remember that it was slower before the changes too.

In the test I'm calling 1000 times an endpoint which is returning list of 1000 items with "self" link. On v1.0.0 it takes ~90s vs ~500ms without links and ~19s with newest 0.25.x. I'm not sure what's the cause of it. Both tests were run against spring 5.1.2.RELEASE.

I'm attaching simple project with the benchmark so you can rerun it yourself. Unpack, and run 'mvn test' should be enough.

spring-hateoas-tests.zip

[odrotbohm]

I think the sample code has to be significantly revamped to produce numbers that we should actually act upon. First of all, it's not a JMH benchmark which means that you're very likely to see distorted numbers driven by JIT warmups, code just being dropped by the JIT as its computed values do not escape methods etc.

Also, I think we can avoid the entire controller execution overhead as we can just declare controller types and then benchmark the direct usage of ControllerEntityLinks. With that, we wouldn't even have to start a Spring Container.

I just gave this a spin and and here were the numbers before I made some optimizations:

• 0.25.1.RELEASE – 18k ops/s
• 1.0.0.BUILD-SNAPSHOT – 14k ops/s

After a few tweaks I got 1.0.0.BUILD-SNAPSHOT – to 40k ops/s, which seemed to be quite neat. Most of that was driven by some optimizations in our affordance building APIs and some improved caching in UriTemplate and MethodParameters. I'll commit those changes against this ticket and will polish up the benchmark later this week so that we have a baseline to monitor the changes in the 1.0 branch.

I've also filed spring-projects/spring-framework#22730 as ReflectionUtils.isObjectMethod(…) also showed up as hotspot during the profiling session.

[mst-avaleo]

Great work Oliver. Yesterday I also started looking deeper into UriTemplate.toString. Though your change is more wider.

Any motivation behind focusing only on EntityLinks? I've checked your changes on EntityLinks and the optimizations are really significant. Though when using WebMvcLinkBuilder it's better but still not acceptable on 1.0.0 (around 2x slower than on 0.25.1)

Any plans to merge the changes to 0.25.x?

[odrotbohm]

As indicated above, I am not very eager to discuss numbers that are not produced by a proper benchmark. The JMH one I have is now at 40k ops/s for 1.0 compared to 18k ops/s on 0.25. That benchmark creates 1000 different links pointing to the same method via the ControllerLinkBuilder APIs directly. I.e. link generation on 1.0 is twice as fast although it does more work.

Can you elaborate what you mean with the focus on EntityLinks? That one hasn't even been touched by the improvements. The biggest chunks of improvement com from MethodParameters caching, the SpringAffordanceBuilder and avoiding superfluous calculations in WebHandler. They all transitively make it into deprecated ControllerLinkBuilder as well as WebMvcLinkBuilder. Quite a bit of it is also contributed by the fix in Spring Framework. I.e. if you use 0.25 on Spring Framework 5.2 snapshots you should see a bit of an improvement, too.

I might be able to backport some of that stuff to 0.25 but it's clearly not a priority at the moment. We're fully focused on 1.0.

[mst-avaleo]

I agree that it's better idea to focus on 1.0.

Regarding focus on EntityLinks. In our setup we are mostly using ControllerLinkBuilder.linkTo(Object invocationValue) method, which does some extra magic with proxies and other stuff. When you test performance thru EntityLinks interface, you will probably not verify this area.

Basically I see degradation of performance when doing following calls from ControllerLinkBuilder:
linkTo(methodOn(ExampleController.class).getExamples(params)).withSelfRel()

I've did some investigation (yet another comparison between 1.x and 0.25.x) and some profiling and I think I found two places which are impacting the most.

DummyInvocationUtils.methodOn(controller, parameters) called from WebMvcLinkBuilder.methodOn(Class<T> controller, Object... parameters) or ControllerLinkBuilder.methodOn(Class<T> controller, Object... parameters)

I think it is caused by removing usage of CLASS_CACHE cache in this commit (avaleo@2a2e0fb) when moving away from OBJENESIS. Do you think similar caching could be reintroduced?

ControllerLinkBuilder.linkTo(Object invocationValue)

It seems that most of the time is currently spent in SpringAffordanceBuilder.create() method. The AFFORDANCES_CACHE is not caching per method (template), since the key is based on final href with final parameters. Do you think we can tweak this caching?

Another option would be to give ability to turn affordance support off, if the app is not using it.

[mst-avaleo]

I found one more spot which impacts significantly.

protected LinkBuilderSupport(UriComponents components, List<Affordance> affordances)

if you change:

		String uriString = components.toUriString();
		UriComponentsBuilder builder = uriString.isEmpty() //
				? UriComponentsBuilder.fromUri(components.toUri()) //
				: UriComponentsBuilder.fromUriString(uriString);

		this.builder = builder;

to

		this.builder = UriComponentsBuilder.fromUri(components.toUri());

Though I'm not sure what was the motivation for the original code. As far as I understand it, the change should be safe.

[mst-avaleo]

Last thing, I'm confirming that changes in ReflectionUtils.isObjectMethod(…) are also significantly improving performance.

[odrotbohm]

I'm closing this for now as we have a couple of improvements committed against that issue and need to ship them in M2 this week.

[mst-avaleo]

Oliver, have you pushed ControllerLinkBuilderBenchmark to the github? I wanted to take a look and maybe try to add the use case with ControllerLinkBuilder.linkTo(Object invocationValue)

[odrotbohm]

I haven't. We have to coordinate a couple of releases and I just wanted to get the improvements we have so far out. Will be on my todo list for next but one week (Spring I/O next week).