Repository navigation
perf: overlap ready task execution with setup - #852
joybestourous wants to merge 13 commits into
Conversation
|
Before reviewing this PR, I see it shows traits of uncurated LLM output, I'd expect a change like this to take an order of magnitude less lines of code. Is there any chance we can narrow it down before investing reviewing time? Another option is to break it down into smaller bits, maybe you want to ship a scoped benchmark first. |
|
Alternatively, you might want to try out Given that |
Start coordinator inputs after their own connection-aware setup and start AQE sampling as each task becomes ready. Keep concrete child destinations in worker plans and preserve task reporting and dynamic-filter stage sealing. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
2514ec1 to
1c74880
Compare
Share task startup across adaptive and static planning. Create report cleanup before startup can be spawned, then transfer ownership into the worker reader so missing reports complete on setup cancellation and response errors. Cover a worker response error after output begins, including metrics and dynamic filter report completion. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Show worker and partition counts in each startup regression and benchmark setup. Keep the shared fixture limited to codec and routing registration. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Configure the existing connection pool with shared pending destinations and query cancellation when building a coordinator-only boundary. Delegate to ordinary boundary execution and remove the remote-stage and task-destination forwarding types. Preserve stream sharing, pool reset behavior, dynamic-filter anchors, and concrete destinations in worker plans. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Store ready and pending worker locations in RemoteStage so ordinary network boundaries can consume tasks independently after routing succeeds. Remove the pending boundary wrapper and separate connection-pool routing state. Keep the dialer selection contract and URL wire format. Document migration from Vec<Url> and reject pending locations when encoding worker plans. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Keep the worker-location API migration beside the routing API changes to avoid an append conflict with newer upgrade-guide entries on main. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
@gabotechs I actually think AQE has this wait as well in the That said, you're right I was being a bit of a meat proxy here, I didn't evaluate the PR well enough. Gave this a major redesign, let me know what you think |
There was a problem hiding this comment.
Thanks @joybestourous! this is looking much better. We have some remote benchmarks that we can use for qualifying this change that use real parquet files in S3 with a real k8s cluster with workers communicating over the network.
Additionally, I think it'd be worth to split this PR into some preliminary refactors + benchmarks before introducing the WorkerLocations, something that comes to mind for the split:
- Refactors like the introduction of the
DynamicPlanBuilder - Add some relevant benchmarks, if any
- The rest of the PR
| #[derive(Clone)] | ||
| pub enum WorkerLocation { | ||
| /// A successfully connected worker selected by the task-routing handler. | ||
| Ready(Url), | ||
| /// A coordinator-consumed task whose routing is still in progress. | ||
| Pending { | ||
| /// The shared result of task setup, including connection-aware routing. | ||
| url: Shared<BoxFuture<'static, Result<Url, Arc<DataFusionError>>>>, | ||
| /// Cancels location resolution and result-stream setup when the query ends. | ||
| query_finished: CancellationToken, | ||
| }, | ||
| } | ||
|
|
There was a problem hiding this comment.
Nice design! this is very elegant
There was a problem hiding this comment.
thank you! 🙏 🙇♀️
| // Iteration zero is warmup. Compare the same test on both revisions. | ||
| #[tokio::test(flavor = "multi_thread", worker_threads = 4)] | ||
| #[ignore = "controlled startup benchmark; run with --release --ignored --nocapture"] | ||
| async fn startup_overlap_benchmark() -> Result<()> { |
There was a problem hiding this comment.
For benchmarks, we use crietrion rather than unit/integration tests. We place those typically in the benches/ folder, and criterion already handles running and reporting of the different benchmarks cases.
I'd anyway try to find one benchmark in which perf improvements from this PR are shown, we have a benchmarking bot that will run the current branch vs main in a remote k8s cluster with real networking, so if we can qualify the improvement with those that'd be great.
If not even the remote benchmarks show an improvement, we can ship a scoped dedicated benchmark as a preliminary PR.
There was a problem hiding this comment.
Added a scoped Criterion benchmark in #873 and removed the ignored timing test here, keeping the correctness tests.
There’s an improvement when ready tasks can do useful work during another task’s setup delay, shortening total query time. The benchmark demonstrates this with a CPU-heavy task and a light task whose routing is delayed by 250 ms. Previously, the CPU work waited for that delay; with the fix, they overlap. Delaying the CPU-heavy task instead leaves the delay on the critical path, and that control shows essentially no improvement. I dont think existing benchmarks demonstrated this, but I do think it's a real pathway
| DynamicPlanBuilder { | ||
| query_coordinator, | ||
| plans_for_viz: &plans_for_viz, | ||
| }, |
There was a problem hiding this comment.
It'd be nice if we could isolate this kind of refactors in a preliminary PR, so that diff in this one is as clean as possible with just the WorkerLocation changes.
For example, I'd create one isolated PR for introducing this DynamicPlanBuilder.
| pub(super) async fn prepare_stages( | ||
| query_coordinator: &Arc<QueryCoordinator>, | ||
| plan: &Arc<dyn ExecutionPlan>, | ||
| defer_head: bool, |
There was a problem hiding this comment.
We should avoid this "if-driven-development" patterns that switch behavior over boolean variables.
If a function needs to switch behavior depending on where it's called, it means that probably this function is just not the right abstraction.
There was a problem hiding this comment.
Agreed, the helper was combining two different preparation paths. Removed the boolean argument and kept static and AQE preparation in their respective modules, accepting some duplication. The common stage-startup lifecycle lives on QueryCoordinator.
| plan: &Arc<dyn ExecutionPlan>, | ||
| defer_head: bool, | ||
| ) -> Result<Arc<dyn ExecutionPlan>> { | ||
| let mut head_stages = HashSet::new(); |
There was a problem hiding this comment.
This does not seem to be using the right terminology in the project.
There can only be exactly one head stage in a query, so the fact that this is plural makes me raise my eyebrow.
There was a problem hiding this comment.
Renamed this to coordinator_input_stage_ids. These identify the stages whose outputs the coordinator consumes; the query still has one head stage.
|
|
||
| /// Worker plans need concrete child destinations. Only inputs consumed directly by the | ||
| /// coordinator can remain pending, letting each task execute after its own setup succeeds. | ||
| pub(super) async fn prepare_stages( |
There was a problem hiding this comment.
Overall, the changes in this file do not look very clear. I see most of the changes here where done so that some code can be reused by the prepare_dynamic_plan.rs file, but it's resulting in some weirdness in the approach.
Rather than aiming for reuse in this PR, I'd rather have some duplicated code in the two paths that can then be factored out in a follow-up if necessary. I would try to keep the diff here as clean as possible even if that implies some duplication.
There was a problem hiding this comment.
Agreed. Removed the shared preparation helper and kept the static and AQE paths separate, accepting some duplication as suggested. Task startup and supervision remain shared on QueryCoordinator, while each preparation path owns its traversal and stage construction.
|
benchmarks run tpch/sf1 |
|
Requested by this comment. Benchmark resultsCompared: PR base === Comparing tpch_parquet/scale_factor=1/max_file_size=134217728/dictionary=pages/sorted=false results 'datafusion-benchmark-base' [prev] with 'datafusion-benchmark-head' [new] === TASKS: prev=139.0, new=139.0, diff=no change (sum of per-query averages) TOTAL: prev=9342 ms, new=9321 ms, diff=1.00 faster ✔ Show full query output q1: prev= 418 ms, new= 299 ms, diff=1.40 faster ✔, tasks: prev=4.0, new=4.0, diff=no change
q2: prev= 266 ms, new= 259 ms, diff=1.03 faster ✔, tasks: prev=0.0, new=0.0, diff=no change
q3: prev= 414 ms, new= 405 ms, diff=1.02 faster ✔, tasks: prev=5.0, new=5.0, diff=no change
q4: prev= 249 ms, new= 248 ms, diff=1.00 faster ✔, tasks: prev=7.0, new=7.0, diff=no change
q5: prev= 475 ms, new= 425 ms, diff=1.12 faster ✔, tasks: prev=10.0, new=10.0, diff=no change
q6: prev= 219 ms, new= 230 ms, diff=1.05 slower ✖, tasks: prev=2.0, new=2.0, diff=no change
q7: prev= 524 ms, new= 514 ms, diff=1.02 faster ✔, tasks: prev=13.0, new=13.0, diff=no change
q8: prev= 574 ms, new= 608 ms, diff=1.06 slower ✖, tasks: prev=15.0, new=15.0, diff=no change
q9: prev= 758 ms, new= 737 ms, diff=1.03 faster ✔, tasks: prev=13.0, new=13.0, diff=no change
q10: prev= 713 ms, new= 694 ms, diff=1.03 faster ✔, tasks: prev=6.0, new=6.0, diff=no change
q11: prev= 239 ms, new= 240 ms, diff=1.00 slower ✖, tasks: prev=0.0, new=0.0, diff=no change
q12: prev= 331 ms, new= 309 ms, diff=1.07 faster ✔, tasks: prev=7.0, new=7.0, diff=no change
q13: prev= 698 ms, new= 698 ms, diff=1.00 slower ✖, tasks: prev=0.0, new=0.0, diff=no change
q14: prev= 263 ms, new= 339 ms, diff=1.29 slower ✖, tasks: prev=5.0, new=5.0, diff=no change
q15: prev= 411 ms, new= 419 ms, diff=1.02 slower ✖, tasks: prev=10.0, new=10.0, diff=no change
q16: prev= 140 ms, new= 146 ms, diff=1.04 slower ✖, tasks: prev=0.0, new=0.0, diff=no change
q17: prev= 417 ms, new= 478 ms, diff=1.15 slower ✖, tasks: prev=7.0, new=7.0, diff=no change
q18: prev= 627 ms, new= 647 ms, diff=1.03 slower ✖, tasks: prev=7.0, new=7.0, diff=no change
q19: prev= 274 ms, new= 264 ms, diff=1.04 faster ✔, tasks: prev=3.0, new=3.0, diff=no change
q20: prev= 526 ms, new= 559 ms, diff=1.06 slower ✖, tasks: prev=8.0, new=8.0, diff=no change
q21: prev= 677 ms, new= 657 ms, diff=1.03 faster ✔, tasks: prev=17.0, new=17.0, diff=no change
q22: prev= 129 ms, new= 146 ms, diff=1.13 slower ✖, tasks: prev=0.0, new=0.0, diff=no change
Verification and run detailsJob
Workload: Capacity: 12 Other timings: Queue 3m 27s · Dataset validation 0s · Total 5m 55s How to use the benchmark botPost a comment whose first non-empty line is:
For example:
Request one or more datasets, without duplicates. The bot checks each one against the dataset bucket before provisioning anything and runs every query in each dataset. Short names predating the parameter-encoded layout, such as
Supported instances and per-worker limits:
Each node runs one worker. The worker allocation reserves 1 vCPU and 4 GiB for Kubernetes and system processes, then makes the rest of the selected instance available to the benchmark. Limits: Only authorized users can enqueue jobs. Jobs run serially, the queue holds 20 active jobs, and each requester may have 3. Query selection is not supported. By default, every query uses 1 warmup and 5 measured iterations for both revisions. |
Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Measure completed queries with a delayed light task, a delayed heavy task, and no setup delay in static and adaptive execution. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Move the existing AQE boundary callback into an implementation of NetworkBoundaryBuilder without changing setup or sampling order. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Remove the shared planner mode switch and name the coordinator input stages directly. Share only supervised task startup, and keep timing measurements in the preliminary Criterion benchmark. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
AQE stage preparation currently lives in a large async closure, which makes subsequent startup changes harder to review. Move that callback into `DynamicPlanBuilder`, implementing the existing `NetworkBoundaryBuilder` trait. Task setup still joins all destinations before sampling kickoff; coalesce behavior, task counts, dynamic filters, statistics and metrics are unchanged. This is a preliminary refactor for #852. Validation: 343 library tests passed, one ignored; formatting passed. The moved callback body was checked for equivalence after accounting for borrowed fields and the removed closure wrappers. Co-authored-by: Codex GPT-6 <noreply@localhost>
Resolve the DynamicPlanBuilder extraction conflict while preserving task startup behavior. Register startup regressions in the unified integration test target. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Task setup skew can postpone useful work on a ready worker. Add a Criterion benchmark for that scheduling case before the execution changes in #852. This PR changes no library behavior and adds no dependencies. The benchmark runs one CPU-heavy task and one light task over localhost gRPC, in both static and AQE modes. Each mode has three cases: no routing delay, a 250 ms routing delay on the light task, and the same delay on the heavy task. It measures execution through complete output collection, excluding worker startup and planning. Every iteration checks output `[0, 1]` and exactly one heavy remote execution. An untimed preflight reports whether CPU work starts and finishes before delayed sibling routing returns. Applied unchanged to #852, the benchmark demonstrates the conditional benefit: | Mode | Case | Baseline | With #852 | Paired change | |---|---|---:|---:|---:| | Static | No delay | 228.15 ms | 227.84 ms | -0.14% | | Static | Delayed light task | 479.29 ms | 294.10 ms | **-38.64%** | | Static | Delayed heavy task | 479.09 ms | 478.60 ms | -0.10% | | AQE | No delay | 228.23 ms | 227.86 ms | -0.16% | | AQE | Delayed light task | 479.37 ms | 294.36 ms | **-38.60%** | | AQE | Delayed heavy task | 479.52 ms | 478.79 ms | -0.15% | In both delayed-light preflights, heavy work started and finished before sibling routing returned with #852; neither witness occurred on the baseline. The controls stayed essentially flat. This demonstrates value when ready useful work can overlap a sibling's setup delay; it does not predict representative S3 or multi-host speedups. Validation: optimized builds and all six cases passed on baseline and candidate. Formatting and Criterion-target Clippy with warnings denied passed. <details> <summary>Measurement scope and reproduction</summary> Measurements used baseline `0291e08` (main `e5d6ea3` plus this benchmark) and candidate source matching #852 at `354d386`. The benchmark and lockfile were identical. The candidate binary was built before the final commit; source hashes were verified against that commit. This was a controlled synthetic experiment on a shared host, with the same four-CPU affinity for both binaries, four Tokio threads, ten Criterion samples per case, one-second warmup and a five-second target measurement window. Three outer pairs were attempted in baseline/candidate, candidate/baseline, baseline/candidate order. Two opposite-order pairs passed the predefined resource gate. The entire third pair was excluded because baseline selected-CPU average utilization p95 was 85.31%, exceeding the 85% threshold; the maximum-utilization threshold was 98%. Raw results for both sides of the excluded pair were retained. Results were not filtered by measured latency. Times above are medians of the accepted per-run Criterion medians; changes are medians of the paired percentage changes. Two outer pairs illustrate this scheduling case, rather than establishing production performance or precise population estimates. ```shell cargo bench -p datafusion-distributed-benchmarks --bench startup_overlap -- --save-baseline main # Run the same benchmark on the candidate revision: cargo bench -p datafusion-distributed-benchmarks --bench startup_overlap -- --baseline main ``` Use separate build directories or rebuild both local packages when switching revisions. Confirm the candidate's delayed-light preflight shows early work and the baseline's does not before interpreting timings. </details> --------- Co-authored-by: Codex GPT-6 <noreply@localhost>
Keep the benchmark documentation beside its source, as merged in datafusion-contrib#873, and remove the duplicate README section from this branch. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
There was a problem hiding this comment.
Thanks @joybestourous!
I get the feeling that most of the complexity in this PR is not necessary, and it's mostly due to LLMs patching things on top of other patches, rather than the changes being properly structured from the ground up.
I'll try to help myself with an actual proposal for structuring things differently, in the meantime, I encourage you to try to get more familiar with the previous structure of the coordinator/ module, and try to come up with simpler alternatives. Historically, LLMs have done a very poor job at contributing there, and almost all the code in that module is handwritten.
EDIT: I'm giving this a try now, and I'm seeing that there is some previous past decisions that are getting in your way, and rethinking them will yield a much simpler final state. Hopefully I'll be able to suggest something concrete soon.
| // Leaf statistics and runtime statistics from sampled network boundaries estimate | ||
| // the compute cost of the next stage. | ||
| let cost = calculate_cost(&input_stage.plan)?; |
There was a problem hiding this comment.
🤔 I see the comment was removed, but it's still relevant, can we bring it back?
| /// become nodes without children and traversing them will not go further down in. | ||
| /// 4. Spawn a background task per worker that waits for the worker to finish and collects | ||
| /// its metrics into [DistributedExec::task_metrics] via the coordinator channel. | ||
| pub(super) async fn prepare_static_plan( |
There was a problem hiding this comment.
Is the deletion of this doc comment intentional? I do think those docs are still relevant.
| head_stage: prepared.await?.data, | ||
| // If the plan was statically planned, the base plan is the same one that will be used for | ||
| // visualization. | ||
| plan_for_viz: Arc::clone(base_plan), |
There was a problem hiding this comment.
I see a bunch of useful comments have been removed in this PR. Unless there's a good reason, I'd try to do a pass recovering lost comments.
| } | ||
|
|
||
| /// Retains annotations when stage preparation replaces nodes without changing topology. | ||
| pub(crate) fn copy_task_counts( |
There was a problem hiding this comment.
🤔 I'd not expect this to be necessary, how is it that it's necessary now but it was not necessary before?
| &self, | ||
| event: RouteTaskEvent<'_>, | ||
| ) -> Option<Result<RouteTaskEventResponse>> { | ||
| Some(self.route(event).await) |
There was a problem hiding this comment.
Why this extra method hop here? why should be good just inlining the contents of route inside the handle method.
| } | ||
| Ok(()) | ||
| }; | ||
| match query_finished { |
There was a problem hiding this comment.
I think we might not need all this query_finished signaling mechanism at all. All of this block is tokio-spawned inside a SpawnedTask, which is a DataFusion helper that already cancels the inner future on Drop.
This means that if a query gets canceled, this SpawnedTask would get dropped, and the inner future is automatically cancelled already.
If I have not missed something, we should be fine to just drop the query_finished mechanism from this PR. If I have missed something, I'd try to anyways remove the additional query_finished cancellation mechanism in favor of using the existing cancellation mechanisms.
| coordinator-to-worker connection is currently established. Implementations can | ||
| choose to retry on other worker if the current one is not available. | ||
|
|
||
| ### Remote stage destinations can still be routing |
There was a problem hiding this comment.
Here, we only declare breaking changes to the public API, but this entry here is documenting a lot more things that are better fitted for a changelog rather than an upgrade guide.
The only thing to mention here is the breaking API change to the Stage::Remote struct that now holds Vec<WorkerLocation> rather than Vec<Url>
| query_coordinator: &Arc<QueryCoordinator>, | ||
| base_plan: &Arc<dyn ExecutionPlan>, | ||
| ) -> Result<PreparedPlan> { | ||
| let mut coordinator_input_stage_ids = HashSet::new(); |
There was a problem hiding this comment.
I don't really get why coordinator input stages are special. Is there anyway we can rearrange this code so that we don't treat those stages as special?
| let workers = if coordinator_input_stage_ids.contains(&stage.num) { | ||
| query_coordinator.start_pending_stage(stage) | ||
| } else { | ||
| let workers = try_join_all( | ||
| (0..stage.tasks).map(|task| query_coordinator.start_task(stage, task)), | ||
| ) | ||
| .await? |
There was a problem hiding this comment.
Ideally, we should not have distinct ways of starting stages depending on where they are located. The process of feeding workers with stage information and establishing bidirectional channels should ideally be the same. It's just not a matter of the increased complexity in the code, it's also that it'd be easier to introduce bugs in the future as we add more things here.
For example, in this code here, you needed to proactively remember to call .seal_dynamic_filter_stage() in the else branch because it's following a different task setup path that happens to not be calling it already.
I'd consider restructuring this code so that it's:
- less convoluted
- less prone to errors from future devs
As a rule of thumb, if you see yourself needing to do major switches of code paths based on if statements, that's a red flag, and the code should probably be revisited.
| ) -> Result<NetworkBoundaryBuilderResult> { | ||
| // A deferred coalesce becomes a concrete destination before another worker stage | ||
| // consumes it. Preserve the planner's pointer-keyed annotations when replacing nodes. | ||
| let prepared = prepare_worker_inputs(self.query_coordinator, &input_stage.plan).await?; |
There was a problem hiding this comment.
I'm seeing that the level of complexity introduced here is just too much.
This needs a new full plan traversal here, the forces a new .copy_task_counts() to patch the things the prepare_worker_inputs did.
All this seems like the classical things LLM would do when they make a change, they realized they screwed up because test do not pass, and then they add patches on top until tests pass.
I'm almost certain there should be ways of doing this with much less complexity.
Task setup postpones ready work: AQE sampler kickoff follows a stage-wide setup join, and coordinator inputs wait for every sibling destination before execution. Start useful work as each task's own routing completes, so it can overlap slower sibling setup.
RemoteStage.workersholds ready or pendingWorkerLocationvalues, consumed through the existing network boundaries and connection pool. Plans sent to workers still contain concrete child destinations, preserving the dialer-based setup failover contract. Query-supervised startup, cancellation and report cleanup cover late errors and early completion; dynamic-filter stages seal after all tasks register. Ready tasks may do work before a sibling later fails; that failure still fails the query and cancels remaining tasks.Static and AQE preparation remain explicit, removing the shared preparation helper and boolean mode argument. Static preparation names the stages consumed by the coordinator
coordinator_input_stage_ids. The shared startup lifecycle operation lives onQueryCoordinator; AQE handles its worker inputs separately.The preliminary
DynamicPlanBuilderrefactor (#872) and Criterion benchmark (#873) have merged into main. This branch includes both; their extraction and benchmark additions no longer appear in the diff. The eight startup correctness regressions remain here.This changes the public Rust type of
RemoteStage.workersfromVec<Url>toVec<WorkerLocation>. The 5.0 upgrade guide explains conversion and resolved-location inspection. The wire format remains URL strings; encoding a pending location fails synchronously.Merge validation at
abbc673: all eight startup regressions passed through main's unified integration target, and formatting passed. The core library source files match354d386; the broader validation and timing results below were collected on that earlier revision.Validation at
354d386: 491 tests passed, three ignored, covering library unit tests, enabled integration targets and doctests withintegration,system-metrics. All eight startup regressions passed, covering static/AQE early output, AQE sampling, nested worker destinations, late failures, cancellation and LIMIT completion. Codec checks cover ready roundtrips and pending rejection. Formatting, all-target library Clippy and Criterion-target Clippy with warnings denied passed. This local run excludes the optional chaos, TPC-H, TPC-DS and ClickBench suites.The new Criterion workload demonstrates useful overlap when a light sibling has a 250 ms routing delay:
Heavy remote work started and finished before delayed sibling routing returned only with the fix. No-delay and delayed-heavy-task controls were essentially flat, with accepted individual pair changes below 0.3%. These are controlled synthetic localhost observations on a shared host: two opposite-order outer pairs passed resource checks; a third pair was excluded for crossing the predefined CPU-contention threshold. #873 records the full table, revisions, methodology and reproduction commands. The result demonstrates conditional scheduling value, not a general production speedup.
Historical TPC-H measurements of an earlier revision
TPC-H SF10 qualification compared baseline
b5701bfwith candidatee96e0a3, using all 22 queries, eight localhost workers and 16 partitions. Exact typed-result comparisons matched schemas, values, multiplicities and SQL ordering at zero floating tolerance. Runtime probes confirmed actual remote output before distinct same-stage sibling readiness in both modes, including AQE sampler output. These differential checks were not an independent TPC-H answer oracle.Six alternating paired full-suite rounds, after one warmup, showed no clear static suite-wide difference (paired median -0.30%, range -1.56% to +1.99%). AQE showed a modest benefit (paired median -2.28%, five of six pairs favoring the candidate, range -4.23% to +0.38%). These measurements concern
e96e0a3, not the current revision, and were limited to localhost workers. They should not be presented as the current revision's representative benchmark qualification.