Skip to content

Keep validation workers supplied with concepts instead of every 10 - #1

Open
dionmcm wants to merge 1 commit into
masterfrom
perf/rule-executor-rolling-workers
Open

Keep validation workers supplied with concepts instead of every 10#1
dionmcm wants to merge 1 commit into
masterfrom
perf/rule-executor-rolling-workers

Conversation

@dionmcm

@dionmcm dionmcm commented Aug 28, 2026

Copy link
Copy Markdown
Member

Branch: perf/rule-executor-rolling-workers · Module: snomed-drools-engine

Problem

RuleExecutor.doValidateComponents built exactly threads (10) single-concept Callables, submitted them with invokeAll, waited for all 10 to finish, cleared the list, and built the next 10.

On a full Snapshot that is 72,240 barriers, each running at the speed of its slowest concept. Per-concept cost varies enormously — a concept with 200 descriptions against one with 2 — so nine threads routinely parked while one finished.

Evidence

Sampling the worker threads during a full-Snapshot run, before the change:

WAITING   75.3%   parked on a condition, no work to do
RUNNABLE  24.7%

Ten threads doing the work of about 2.5. After the change, same workload:

RUNNABLE  99.4%
BLOCKED    0.6%

Change

Start threads long-lived workers once. Each keeps its own StatelessKieSession list — unchanged ownership, so no new sharing — and claims the next concept from a shared AtomicInteger until the list is exhausted. Also removes 722,404 Callable allocations.

Two smaller fixes that belong with it:

  • exceptionContents is now a synchronized list. Workers already appended to it concurrently on the old code path; with a plain ArrayList a validation error raised by two concepts at once could be lost or corrupt the list.
  • runTasks now calls Future.get(), so a worker that dies from an unexpected throwable fails the run instead of silently reducing parallelism. Previously invokeAll's results were discarded.

Test

RuleExecutorTest.testWorkersClaimNextConceptWithoutBatchBarrier builds 11 concepts where concept 1 blocks until concept 11 has been visited. Under the old batching, concept 11 is in the second batch of 10 and can never be reached, so the test deadlocks to its timeout and fails. It passes only when a free worker is allowed to run ahead. Verified failing before the change, passing after.

Notes for review

Concept ordering within the run is no longer deterministic between threads. It was not deterministic before either — 10 concepts ran concurrently — and findings are per-concept, which the identical-TSV result confirms.


About this series

One of five independent performance changes to the Drools validation path, measured against a real 722,404-concept AU edition Snapshot. Each is a single commit on master, builds and passes the full suite alone, and leaves the findings byte-identical: 10,949 before and after, with matching report TSVs.

Together they take a full-Snapshot validation from 636s to 118s.

This PR and perf/scale-validation-workers touch adjacent lines; whichever merges second needs a one-line rebase. There is no logical dependency and the order does not matter.

@dionmcm dionmcm changed the title Keep validation workers supplied with concepts instead of barriering every 10 Keep validation workers supplied with concepts instead of every 10 Aug 28, 2026
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

Successfully merging this pull request may close these issues.

1 participant