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

ControllerLinkBuilder.linkTo(…) still slow #698

Closed
MrBuddyCasino opened this issue Feb 14, 2018 · 9 comments
Closed

ControllerLinkBuilder.linkTo(…) still slow #698

MrBuddyCasino opened this issue Feb 14, 2018 · 9 comments

Comments

@MrBuddyCasino
Copy link

We're on 1.5.10.RELEASE (=Spring Core 4.3.14) and spring-hateoas 0.24.0.RELEASE. Profiling shows that adding the links takes more time than even our database queries (18.5 ms linkTo() vs. 8.13ms JDBC call).

I've seen issue #511 and it seems those improvements have already been incorporated in the versions that we use?

@odrotbohm
Copy link
Member

Care to provide a sample project that shows the slowness?

@MrBuddyCasino
Copy link
Author

Sure, I uploaded a stripped-down version to: https://github.com/MrBuddyCasino/spring-hateoas-demo

You'll find that linkTo() takes about 17ms.

@odrotbohm
Copy link
Member

A single call to a server does not produce actionable numbers as they might be influenced by all kinds of JVM factors. Is there a chance we can get this into a more approachable fashion? A JMH benchmark or at least a test case that doesn't involve a server, a network call or the like? I am pretty sure we don't need anything actuator, Swagger or DevTools to reproduce the numbers, do we?

@MrBuddyCasino
Copy link
Author

Running the test in a loop and accounting for JIT and stuff doesn't change anything, the numbers stay constant over many requests, except the first few ones obviously.

However, I found out that there are big differences depending on how the app is started - in one case there are ~17ms, in another 1-2ms, which is ok but still not great. I have yet to figure out what exactly causes this, it might be a Tomcat thing.

@MrBuddyCasino
Copy link
Author

Alright, the joke is on me - apparently attaching the profiler had an outsized effect on linkTo() relative to the rest of the application, probably due to reflection de-optimizations.

We'll still move to custom link generation, as spending 1-3 ms for generating two links is a bit on the high side, and we have instances where we need to generate many more (don't ask, stupid requirement). I looked at the implementation and the code is doing a lot - I understand why, but its probably not easy to speed up.

@odrotbohm
Copy link
Member

Hm, okay. My point was basically that "linkTo(…) takes 17ms" is not something we can actually act on. If you are already profiling this, you should also be able to see where within linkTo(…) the time is spent. But without a benchmark project that I can run, there's hardly anything I can really do.

@Dretch
Copy link

Dretch commented Dec 5, 2019

I also have an issue with link generation being slow. Specifically WebMvcLinkBuilder::methodOn seems to occasionally take > 100 ms, despite normally taking ~1 ms.

I have not been able to reproduce the slowness in a simple test, unfortunately, but I have found an effective workaround - I am using code similar to the below to cache the results of WebMvcLinkBuilder::methodOn. A thread-local cache is used because I encountered what I guess to be thread-safety issues when sharing the returned proxies between threads.

public class CachingWebMvcLinkBuilder {

    private static final ThreadLocal<Map<Class<?>, Object>> CACHE = ThreadLocal.withInitial(HashMap::new);

    public static <T> T methodOn(Class<T> controller) {
        // noinspection unchecked
        return (T) CACHE.get().computeIfAbsent(controller, WebMvcLinkBuilder::methodOn);
    }
}

I realise this probably doesn't give enough information to resolve the issue, but perhaps this workaround will be useful to someone else.

@odrotbohm
Copy link
Member

@Dretch – Interesting stuff. Your idea spiked a bit of work that got the entire link creation almost 3 times as fast as before. Will be committed against #1148.

@Dretch
Copy link

Dretch commented Dec 10, 2019

Thanks @odrotbohm - that looks awesome. I'll try upgrading when a new version is released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants