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

Caffeine pins the carrier thread of virtual threads #779

Closed
imperatorx opened this issue Sep 26, 2022 · 4 comments
Closed

Caffeine pins the carrier thread of virtual threads #779

imperatorx opened this issue Sep 26, 2022 · 4 comments

Comments

@imperatorx
Copy link

imperatorx commented Sep 26, 2022

When running on virtual threads, one must not be doing blocking IO in synchronized blocks, or in a native function. However, Caffine uses ConcurrentHashMap's compute function, which operates with synchonized blocks. The following code yields a carrier thread pin event:

 java.base/java.util.concurrent.ConcurrentHashMap.compute(ConcurrentHashMap.java:1916) <== monitors:1
@Test
  public void foo() {
      // compile and run with --enable-preview --add-modules jdk.incubator.concurrent -Djdk.tracePinnedThreads=full
      var cache = Caffeine.newBuilder()
              .expireAfterWrite(10, TimeUnit.SECONDS)
              .build(new CacheLoader<String, String>() {
                  @Override
                  public String load(String key) throws Exception {
                      try {
                          // Simulate IO with sleep
                          Thread.sleep(10000L);
                          return "foo";
                      } catch (InterruptedException e) {
                          throw new RuntimeException(e);
                      }
                  }
              });

      try (var scope = new StructuredTaskScope.ShutdownOnFailure()) {
          for(int i = 0; i < 2; i++) {
              scope.fork(() -> cache.get("foo"));
          }

          scope.join();
          scope.throwIfFailed();
      } catch (Exception e) {
          throw new RuntimeException(e);
      }
  }
@ben-manes
Copy link
Owner

Please try using AsyncCache which performs the work on a CompletableFuture. This would only synchronize for the short, non-IO needs to maintain consistency with entry updates in ConcurrentHashMap. The actual expensive work should not be done under a synchronized block and instead parks and unparks threads via LockSupport. You can use Caffeine.executor to set to the virtual task executor.

@imperatorx
Copy link
Author

Thanks, the following style is working:

@Test
    public void foo() {
        // compile and run with --enable-preview --add-modules jdk.incubator.concurrent -Djdk.tracePinnedThreads=full
        var cache = Caffeine.newBuilder()
                .expireAfterWrite(10, TimeUnit.SECONDS)
                .executor(Executors.newVirtualThreadPerTaskExecutor())
                .buildAsync(new CacheLoader<String, String>() {
                    @Override
                    public String load(String key) throws Exception {
                        try {
                            // Simulate IO with sleep
                            Thread.sleep(5000L);
                            return "foo";
                        } catch (InterruptedException e) {
                            throw new RuntimeException(e);
                        }
                    }
                });

        try (var scope = new StructuredTaskScope.ShutdownOnFailure()) {
            for (int i = 0; i < 2; i++) {
                scope.fork(() -> cache.get("foo").join());
            }

            scope.join();
            scope.throwIfFailed();
        } catch (Exception e) {
            throw new RuntimeException(e);
        }
    }

@qyyxl
Copy link

qyyxl commented Jul 30, 2024

 When I use the following method:

```
caffeine.buildAsync(new AsyncCacheLoader<>()  { ···  }
```

 I encounter a stack trace that leads to a pinned issue:

```
com.github.benmanes.caffeine.cache.BoundedLocalCache.lambda$refreshIfNeeded$3(BoundedLocalCache.java:1333)
java.base/java.util.concurrent.ConcurrentHashMap.computeIfAbsent(ConcurrentHashMap.java:1708) <== monitors:1
com.github.benmanes.caffeine.cache.BoundedLocalCache.refreshIfNeeded(BoundedLocalCache.java:1325)
com.github.benmanes.caffeine.cache.BoundedLocalCache.afterRead(BoundedLocalCache.java:1290)
com.github.benmanes.caffeine.cache.BoundedLocalCache.computeIfAbsent(BoundedLocalCache.java:2661)
com.github.benmanes.caffeine.cache.LocalAsyncCache.get(LocalAsyncCache.java:92)
com.github.benmanes.caffeine.cache.LocalAsyncCache.get(LocalAsyncCache.java:83)
com.github.benmanes.caffeine.cache.LocalAsyncLoadingCache.get(LocalAsyncLoadingCache.java:133)
```

@ben-manes
Copy link
Owner

Unfortunately the only reliable solution will be when virtual threads support object monitors. The recent Loom EA builds handle this, so maybe JDK 25 will rectify this problem.

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

3 participants