Spring-hateoas: Memory pressure when creating Link instances

Created on 7 May 2020  Â·  12Comments  Â·  Source: spring-projects/spring-hateoas

There is a memory leak, when creating a link and adding it to the collection. Checked memory leak both in JProfiler, using runtime.freeMemory(). Attaching code as the whole example is 50 lines of code.
After executing this, I get 3gb hanging without being GCed. I did the same test just with string and memory is GCd.

java public class MainClass { public static void main(String ... args) throws InterruptedException { addItemsToList(10000); //100 mb gc_and_report(); addItemsToList(50000); // 500mb gc_and_report(); addItemsToList(100000); // 1gb gc_and_report(); addItemsToList(500000); // 5gb gc_and_report(); Thread.sleep(10000000); } //size == 10kb * nItems //After the method has finished the memory should be GC private static void addItemsToList(int nItems) { List<Link> items = new ArrayList<>(); for (int i=0;i<nItems;i++){ items.add(createLink()); } System.out.println(nItems + "items added"); } //approximately 10kb link private static Link createLink() { return new Link(generateString(2500), IanaLinkRelations.SELF); } private static final String generateString(int nChars) { int leftLimit = 97; // letter 'a' int rightLimit = 122; // letter 'z' Random random = new Random(); return random.ints(leftLimit, rightLimit + 1) .limit(nChars) .collect(StringBuilder::new, StringBuilder::appendCodePoint, StringBuilder::append) .toString(); } private static void gc_and_report() { Runtime runtime = Runtime.getRuntime(); System.gc(); long usedMemoryMB = (runtime.totalMemory() - runtime.freeMemory())/(1024L * 1024L); System.out.println(String.format("Used memory in mb: %d",usedMemoryMB)); } }

Configuraiton:
java version: 11
spring-hateoas version 1.0.4.RELEASE

bug waiting for feedback performance core

Most helpful comment

BTW: Your replacement code simply skips the cache as the condition always evaluates to false
Yeah, I've noticed it few minutes later, sorry for the churn.
Our move to a ConcurrentReferenceHashMap
I doubled check, I still get OutOfMemoryError exception.

Anyway, as we now know the reason of the problem, what would be you proposal, how to overcome it?
Can you at least make CACHE public, to just give an opportunity for users to clean it manually?
Or have some method to set a limit for the cache or a method to clean it somehow?

All 12 comments

Would you mind checking whether this still exists in 1.0.5 or 1.1 snapshots?

Issue remains with 1.0.5
Issue remains with 1.1.0.RC1 (I've installed it from zip from releases tab as it's not published to public mvn)

As a side note, I've used RandomStringUtils.randomAlphanumeric for generating the random string from apache commons library, the issue remains. I just made a very simple example, that is not using any other dependencies.

Seems like the problem is coming from cache.
If I disable cache logic here
By just calling constructor explicitly, like return new UriTemplate(template) there is no memory leak.

UPD:
The problem comes from the fact that CACHE is static, so it's actually never cleaned. We need to somehow force cleaning the cache.

I know, I know. Our move to a ConcurrentReferenceHashMap for that was an attempt to at least react to garbage collection pressure. The cache was introduced to avoid having to parse every source string all the time as we assume the sources handed into them method would saturate as an application usually does not produce gigabytes of different URIs but lets say a couple of thousands, most of them looking exactly the same all the time. If I remove the cache, that causes around 30% in throughput decrease for simple link creation in benchmarks.

We're looking for a capped cache implementation within Spring Framework globally as that topic comes up in a few different places actually. I'd like to table this until we find a more global solution.

BTW: Your replacement code simply skips the cache as the condition always evaluates to false.

BTW: Your replacement code simply skips the cache as the condition always evaluates to false
Yeah, I've noticed it few minutes later, sorry for the churn.
Our move to a ConcurrentReferenceHashMap
I doubled check, I still get OutOfMemoryError exception.

Anyway, as we now know the reason of the problem, what would be you proposal, how to overcome it?
Can you at least make CACHE public, to just give an opportunity for users to clean it manually?
Or have some method to set a limit for the cache or a method to clean it somehow?

Hi everybody,

our service (9 nodes each ~15000 requests per minute) suffered performance issues as well.
We're building all our urls with spring-hateoas internally.

The urls we build are highly diverse. This comes from the fact that the service itself has many endpoints and that each endpoint has a high degree of freedom in terms of input parameters because of text search queries as request parameters.
From our point of view there is almost an infinite volume of urls to build.

Beside of the regular user traffic we also have to deal with a high amount of bot peak traffic.

When we started investigating the problem we came across this issue and had a deeper look into this ConcurrentReferenceHashMap approach.

spring-hateoas currently uses the default constructor of ConcurrentReferenceHashMap which has an initial capacity of 16, and a load factor 0.75.

Our assumption was that because we have so many diverse urls the cache will never have a satisfactory cache ratio.
Additionally, it will only be cleared during GC cycles.

The other thing we had in mind was the load factor.
Assuming because of the high variation of the urls all requests to UriTemplate resulted in a new entry in the ConcurrentReferenceHashMap the load factor might be to small for us.

To validate our assumptions we monkey patched (copied UriTemplate into our project) UriTemplate and replaced the cache implementation (in our case a Caffeine loading cache).

public class UriTemplate implements Iterable<TemplateVariable>, Serializable {

    // ...

    private static final LoadingCache<String, UriTemplate> CACHE = Caffeine.newBuilder()
            .maximumSize(CACHE_SIZE)
            .build(UriTemplate::new);

    // ...
}

We're live with this approach for a couple of days, and we do not recognize any major performance issues.
So maybe this workaround helps others as well.

With this comment we mainly want to contribute our perspective in terms of assumptions regarding url diversity and load peaks.
We also want to draw some attention to this issue because we're not that much happy about applying such a dirty hack to such a cool project! :)

Hi,

We also ran into a growing memory usage and performance issues on our service due to this. Old Gen memory started growing and the garbage collector (G1/Java8) started collecting continuously, impacting performance. I guess it wouldn't have caused an OOM as these are soft references, but the garbage collector goes into "panic mode" long before that and is still not able to free the soft references.

Running a jmap -histo:live on the process revealed the heap mainly consisted of 7 million live org.springframework.util.ConcurrentReferenceHashMap$SoftEntryReference instances and all the objects these entries point to (UriTemplate, DefaultUriBuilderFactory, String...).

We actually thought about something similar to @chrgue's solution, but instead chose to tweak the JVM flag SoftRefLRUPolicyMSPerMB, which controls how long soft references are kept live. I.e -XX:SoftRefLRUPolicyMSPerMB=250 (default is 1000ms). We have been running with this flag for a couple of days and it seems to have resolved the issue at least temporarily.

This solution is of course risky in case other parts of the application rely on soft references, as the flag affects the whole JVM. It also doesn't solve the unpredictable nature of the cache.

We also like to highlight that this is a real issue that requires some not-so-pretty workarounds. Hopefully there is some clean solution that would fit both the project and us users.

Thanks for a great project!

@chrgue I am not that much expert when it comes to Spring Hateoas library. We use [WebMvcLinkBuilder.linkTo](https://github.com/spring-projects/spring-hateoas/blob/887b93aabacab7256e0b4860f40ab2ba6c3374bb/src/main/java/org/springframework/hateoas/server/mvc/WebMvcLinkBuilder.java#L78-L78) to create links. I know that it uses UriTemplateBuilder internally which in turn stores the UriTemplate objects in Cache.

final Link link = WebMvcLinkBuilder.linkTo(Controller.class).withSelfRel();

I am not sure how I can change my code to make it use the new Caffeine cache as you mentioned in you comment. It would be really helpful if you can provide more details on how you "monkey patched" or made changes to make spring hateoas to use Caffeine as Cache.

I just pushed an intermediate fix for the leak to 1.2.0-SNAPSHOT. Please give the snapshots a try. If that solves the problem for you folks, I'd consider to backport this to 1.1.x.

Thanks for the intermediate fix @odrotbohm ! In our case the 1024 size of the cache will mean a cache hit rate close to 0. Still, I think the performance overhead for this would be acceptable.

What worries me, if I'm not mistaken, is that the Link creation is now behind a single WriteLock, and as the cache hit rate will be almost 0, every link will be recreated each time. With the amount of links we are creating I believe this will cause lock contention with our workload. So my gut feeling is that our current workaround using JVM flags is faster and more tuneable than this.

That said, let's see if we can test your fix. It's a bit tricky to simulate the correct amount of different links using synthetic testing of our app though.

I had a conversation with @mp911de this afternoon and we're going to take an entirely different route: we're gonna remove the cache from UriTemplate completely and rather refactor Link not to even bother creating a template if none is needed (i.e. the source string not contain any curly brace in the first place). The expansion methods work perfectly fine if the template is not present and if someone is ending up calling getTemplate(…) we can still lazily create one.

That said, the memory "leak" presented in the original code snippet above is not really even caused by UriTemplate itself but the fact that the generated links are added to a list that's held for the entire time of processing. As the individual links keep references to the UriTemplate instances, the GC will never cause the soft references in the cache to be wiped as they're still referenced.

I.e. what's being simulated is not a huge number of requests but a huge number of links generated in a single request. I think it's highly unlikely that this resembles a typical application scenario. I'd love to learn how others that reported the problem as well are handling the link instances created. If they're without reference after e.g a request has been processed the ConcurrentReferenceHashMap should just do its job fine.

Was this page helpful?
0 / 5 - 0 ratings