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

Entries are not cleaned when using weakValues #111

Closed
tepitebson opened this issue Aug 10, 2016 · 6 comments
Closed

Entries are not cleaned when using weakValues #111

tepitebson opened this issue Aug 10, 2016 · 6 comments

Comments

@tepitebson
Copy link

We faced a memory leak after upgrading caffeine from 2.2.7 to 2.3.1. It seems that cache entries are not evicted when using weak values. Following test is working on 2.2.7 but eats all memory with 2.3.1:

@Test
public void testWeakValuesEviction() throws InterruptedException {

    Cache<Long, Object> cache = Caffeine.newBuilder().executor(e -> e.run()).weakValues().recordStats().build();
    ScheduledThreadPoolExecutor exec = new ScheduledThreadPoolExecutor(1);
    AtomicLong versionCounter = new AtomicLong(1L);
    exec.scheduleAtFixedRate(() -> {
        int idCounter = 1;
        while (idCounter++ < 200_000) {
            cache.put(versionCounter.getAndIncrement(), new Object());
        }

    } , 0L, 500L, TimeUnit.MILLISECONDS);
    for (;;) {
        Thread.sleep(10000);
        CacheStats stats = cache.stats();
        System.out.println(stats.toString());
        // cache.cleanUp();
        System.gc();

    }
}

Eviction count stays at zero on 2.3.1 while growing fast on 2.2.7. If cache is cleaned up explicitly from outside, entries are evicted.

@ben-manes
Copy link
Owner

I'm sorry to hear this. I ran your test and see the same surprising behavior. Yet when I debug ReferenceTest#put(), I do see some collection occurring. I'm not sure off-hand what recent change would cause a regression for your test case.

I'll keep exploring and if we find the cause, try to have a bug fix released by Sunday evening.

@ben-manes
Copy link
Owner

Sorry, I accidentally debugged the counts with the Guava provider which of course worked. The Caffeine provider passed because the assertion's count isn't restrictive enough, despite the cache not having evicted any entries. As you said, if I manually cleanUp then it discards.

@ben-manes
Copy link
Owner

I found the problem in BoundedLocalCache#afterWrite().

This was incorrectly modified to not call scheduleAfterWrite() if buffersWrites() is false. The surrounding logic was changed to use a bounded write-buffer when investigating what turned out to be the JVM bug Missed submissions in ForkJoinPool. The afterWrite() method had to then handle back-pressure due to a full queue, which typically only happens in synthetic tests. Accidentally the scheduleAfterWrite() was moved into the if-block, whereas in 2.2.7 it is called regardless.

The maintenance work should still occur after a number of reads exceeds a threshold. I'd like to shore up the ReferenceTest assertions prior to releasing this fix.

@tepitebson
Copy link
Author

Hi. Good to see that the problem has been located, we will revert to 2.3.0 meanwhile since that version does not seem to be affected by this issue. Thank you for your swift response for the issue.

@ben-manes
Copy link
Owner

Yes, it looks like I introduced it in 2.3.1 only. I'm working on improvements to the unit tests and hope to release this evening, but it is already getting late. I might do so tomorrow.

@ben-manes
Copy link
Owner

Released. Sorry about any trouble this caused and thanks for understanding.

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