Skip to content

ScheduledDataLoaderRegistry ticker mode dispatches every registered data loader on each dispatchAll(), even when nothing is queued #292

Open

Description

@Meemaw

Summary

In ticker mode, ScheduledDataLoaderRegistry.dispatchAll() runs a full dispatch() on every registered data loader, including loaders with nothing queued. Each empty dispatch still:

  • does a CAS on the loader queue;
  • reads the clock and writes lastDispatchTime;
  • calls instrumentation().beginDispatch(...);
  • attaches the end-of-dispatch callback to an empty result, allocating about 48 bytes per loader per call.

The registry also evaluates the predicate and re-arms the ticker once per loader.

GraphQL servers typically register every loader the application defines in each request's registry, but a request queues keys on only a few of them. graphql-java's per-level dispatch then calls dispatchAll() many times per request, and the ticker calls it again every schedule. Nearly all of that work goes to loaders with nothing to dispatch, so the cost grows with registered loaders × dispatchAll() calls rather than with actual batches.

Version: master at da59d1d, which includes #291, and 6.0.0. We hit this through Netflix DGS 12 in ticker mode with graphql-java 25.

Where

ScheduledDataLoaderRegistry (master):

public int dispatchAllWithCount() {
    int sum = 0;
    for (Map.Entry<String, DataLoader<?, ?>> entry : dataLoaders.entrySet()) {
        ...
        sum += dispatchOrReschedule(key, dataLoader);   // shouldDispatch + dispatchWithCounts() for every loader
    }
    return sum;
}

and DataLoaderHelper.dispatch(), which runs in full even when the queue is empty:

DataLoaderInstrumentationContext<DispatchResult<?>> instrCtx = ctxOrNoopCtx(instrumentation().beginDispatch(dataLoader));
...
if (loaderQueueEntryHead == null) {
    lastDispatchTime.set(now());
    instrCtx.onDispatched();
    return endDispatchCtx(instrCtx, emptyDispatchResult());
}

Measurements

JMH, in-repo. The proposed PR adds ScheduledDataLoaderRegistryDispatchPerformance. It builds a ticker-mode registry the way DGS does: DISPATCH_NEVER for the registry, DISPATCH_ALWAYS per loader, a schedule long enough that the ticker never fires during a measurement, and caching disabled so every load is queued. Each operation queues one key on N loaders, then calls dispatchAllWithCount().

Runs were on JDK 21, Apple M-series, -prof gc, two interleaved runs per variant. The figures below are from master at da59d1d:

Registered loaders Loaders with queued keys Time / dispatchAll Allocated / dispatchAll
20 0 853–881 ns 960 B
200 0 9,199–9,468 ns 9,600 B
20 5 1,129–1,199 ns 3,920 B
200 5 9,766–10,069 ns 16,600–16,880 B

With 5 loaders doing real work, the 195 idle loaders account for about 85% of the time and 83% of the allocation.

Production. In a GraphQL service with about 190 registered loaders, CPU profiles across 12 instances (154k samples) showed dispatchAll() visiting loaders with empty queues at about 3.2% of total CPU:

Where the empty-dispatch cost went Share of total CPU
Empty DataLoaderHelper.dispatch(): CAS, clock.instant(), lastDispatchTime, instrumentation, endDispatchCtx ~1.6%
Iterating the ConcurrentHashMap of loaders ~1.1%
shouldDispatch predicate-map lookups ~0.35%

About 62% of those calls came from graphql-java's per-level dispatch and 38% from the ticker. Code that only runs for a non-empty queue took about 0.03% of CPU, against about 0.87% for the empty path. So fewer than 3% of loader visits had anything to dispatch.

Proposal

In ticker mode, skip loaders whose dispatchDepth() is 0, and re-arm the ticker once per dispatchAll() instead of once per loader. dispatchDepth() is a single atomic read of the queue head.

A key queued after the check is dispatched by the next tick, which is the same window as a key queued just after a dispatch today. The ticker contract is unchanged: after the first dispatchAll(), every loader whose predicate passes is dispatched at least every schedule until close().

The observable differences, both on idle loaders only:

  • lastDispatchTime is no longer refreshed, which matters only for time-based predicates such as DispatchPredicate.dispatchIfLongerThan;
  • beginDispatch no longer fires for loaders with nothing to dispatch.

Non-ticker mode would stay as it is: there, a not-yet-dispatched loader relies on its per-loader reschedule chain.

I have a PR ready with a test and the JMH benchmark.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions