PerformCleanupTask

[severous]

Thanks for your excellent work. I hava a little confusion about the design of BoundedLocalCache.PerformCleanupTask.

• The exec method of ForkJoinTask returns false meaning that PerformCleanupTask never stops. So is it necessary to trigger the PerformCleanupTask again for every cache write?
• The execution of performCleanupTask requires simultaneously checking both drainStatus and evictionLock, as seen in methods BoundedLocalCache#scheduleDrainBuffers and BoundedLocalCache#scheduleAfterWrite. Could using a stronger memory ordering mode for drainStatus instead of opaque mode potentially eliminate the need for evictionLock?

  void scheduleAfterWrite() {
    @Var int drainStatus = drainStatusOpaque();
    for (;;) {
      switch (drainStatus) {
        case IDLE:
          casDrainStatus(IDLE, REQUIRED);
          scheduleDrainBuffers();
          return;
        case REQUIRED:
          scheduleDrainBuffers();
          return;
        case PROCESSING_TO_IDLE:
          if (casDrainStatus(PROCESSING_TO_IDLE, PROCESSING_TO_REQUIRED)) {
            return;
          }
          drainStatus = drainStatusAcquire();
          continue;
        case PROCESSING_TO_REQUIRED:
          return;
        default:
          throw new IllegalStateException("Invalid drain status: " + drainStatus);
      }
    }
  }
  
    void scheduleDrainBuffers() {
    if (drainStatusOpaque() >= PROCESSING_TO_IDLE) {
      return;
    }
    if (evictionLock.tryLock()) {
      try {
        int drainStatus = drainStatusOpaque();
        if (drainStatus >= PROCESSING_TO_IDLE) {
          return;
        }
        setDrainStatusRelease(PROCESSING_TO_IDLE);
        executor.execute(drainBuffersTask);
      } catch (Throwable t) {
        logger.log(Level.WARNING, "Exception thrown when submitting maintenance task", t);
        maintenance(/* ignored */ null);
      } finally {
        evictionLock.unlock();
      }
    }
  }

[ben-manes]

The exec method of ForkJoinTask returns false meaning that PerformCleanupTask never stops. So is it necessary to trigger the PerformCleanupTask again for every cache write?

ForkJoinTask is a future so it has a concept of completion which the internal doExec() method checks before it will call exec(). When returning false we indicate that there is a reschedule strategy so that the task may be invoked again if re-submitted to the pool. In FJP's case this is primarily for CountedCompleter while In ours we want each submission of the task to try an execute once and never have the task instance be considered completed. This is because our usage is purely an optimization to avoid an allocation when a Runnable is submitted to the pool where it will be wrapped by an ForkJoinPool$AdaptedRunnable. When profiling in early versions this showed up in as a memory hotspot, which while inexpensive added some GC pressure. By extending the task type we can avoid introducing that overhead as its allocated once and reused.

The execution of performCleanupTask requires simultaneously checking both drainStatus and evictionLock, as seen in methods BoundedLocalCache#scheduleDrainBuffers and BoundedLocalCache#scheduleAfterWrite. Could using a stronger memory ordering mode for drainStatus instead of opaque mode potentially eliminate the need for evictionLock?

A stronger memory ordering alone would not be enough as there would be races of multiple writes observing the REQUIRED state and trying to drain. One could combine the drainStatus into the lock states using a custom AbstractQueuedSynchronizer, but that is complexity without an as of yet observed performance concern. The drainStatus primarily serves to avoid unnecessary tryLock attempts like a TTAS lock to avoid unnecessary CAS operations when trying to acquire the lock.

We don't want to treat the drainStatus as a binary semaphore where we handoff exclusive access to the scheduled thread. That relies too favorably on the executor which could be starved or fail to actually attempt to perform the work (both have happened due to FJP bugs). When the write buffer is full then the writers barge the lock trying to assist, which induces back pressure if writes outpace evictions in a synthetic load test. An explicit Cache.cleanUp() will block on the lock and run maintenance, as do some of the Cache.policy() methods when they adjust policy settings. This best-effort and non-exclusive nature to task scheduling allows for some redundant (but fast) executions while minimizing wait times and being more resilient to bugs or hiccups.

[severous]

Thanks for you reply.

The scheduling principle of ForkJoinPool is too complicated, and I don't know much about it. What I don't understand is why returning false can ensure that each submitted task is attempted to be executed once? After writing the cache item, PerformCleanupTask will be submitted again.

[ben-manes]

False only avoids the task from not running when submitting, as otherwise the task will no-op or we need to allocate a new task per submission. The FJP task is a runnable future.

[ben-manes]

The pool doesn’t resubmit or rerun the task when false. It’s a pending future and false just indicates it’s not done yet so the current results are not final. This allows for concurrent recursive and asynchronous child tasks, which is fancy but not our need. We just want the task to run once per submission and never reach a terminal state.