Repository navigation
Conversation
`Tracker::propagate_all` visits locations by strongly connected component, in a topological order of the components computed when the tracker is built, rather than by time and location. A location outside any cycle is visited once per call, after every change that can reach it, applies all its changes together and pushes its net changes directly. The locations of a component with a cycle are visited in order of time, as before. Pending changes are kept per location, so consolidating them sorts each location's few times rather than all changes by location and time. `Tracker::pushed` now returns a `Vec` of net changes, consolidated per location and in no particular order.
|
Have had a read, and want to share some thoughts.
So a bit of TBD thinking for me to do about whether this is the layer to tidy up graphs that others have built. We've concluded "no" in the past, but if it's a thing that cannot be worked around then it becomes timely's responsibility. I'm not clear on the whole stack you have, and where it is ok to interpose, but that might be helpful to unstick you. For example, DD specifically handles fine grained update timestamps (the 20,000 and 100,000 I think?) as distinct from coarser timestamps it hands to timely, for the reason that the progress tracking pays in terms of timestamp granularity. It sounds like this could be a fix at the FlowLog layer, unless something is forcing a large number of distinct timely timestamps. |
|
Thanks for reading it closely. FlowLog builds against a vendored copy of timely with this patch applied, so there's no urgency on our side; take whatever time you need to think it through. If it would make review easier, I can split the PR into two:
You asked whether this could be fixed in FlowLog instead. I don't think so. I'll share some context, then results of a quick experiment. What the benchmark measuresIn How FlowLog structures its dataflowsFlowLog already gives each recursive stratum its own iterative scope. The incident program's root scope has 279 locations, and 29 of them form a cycle. That cycle isn't a recursive Datalog stratum. It comes from a relation the program reads one event instant prior, implemented as a Why the root scope sees so many roundsFlowLog doesn't create extra distinct timestamps. At 1 s commits, each commit takes about 3.1 rounds of progress tracking. What costs time is the work in each round: the tracker walks the root's worklist in Committing less often would reduce the number of rounds. At 60 s commits, this PR's gain on our workloads falls to about 1%, which agrees with your point about timestamp granularity. But 60 s is almost two orders of magnitude worse latency than 1 s. Our workloads are soft real-time, so latency is an operational problem for us. Longer commit intervals also cause other problems, such as memory pressure from the updates that build up between commits. Putting the cycle in a regionTo test your suggestion, I moved exactly that cycle (the variable, the three rules that close it, and their unions) into a
The region doesn't help, because most of the tracker's work is in the roughly 250 root locations outside the cycle. A region removes the cycle from the root's tracker, but those 250 locations remain, and they're the expensive part. |
|
Thanks for the context. One confusing bit, that Claude articulated better than I was able to, is
Our read is that these numbers seem like they might be about optimizing a largely idle system. When there is little data to put in the data plane, the fixed costs of the control plane dominate. This will also get worse with more workers, but .. from experience .. it vanishes (relatively) as you fill up the data plane. It's meant to be a fixed tax, not unlike spinning up a thread; useful, but only if you have enough work to pay against the overhead. If the system is largely idle, it's expected that progress tracking will be the dominant cost. I think the other work we've discussed on avoiding scheduling operators and regions that hold no capabilities may be more impactful, in that case. That is the step that gives the order of magnitude reduction when parts of the dataflow are idle; whole blocks aren't even investigated, until data actually arrive. Let me know if that is roughly accurate, that is whether the FlowLog times you are seeing spent in progress tracking are on a loaded system, or whether it's largely not busy. E.g. how many absolute milliseconds per second of non-progress work are you seeing each second (and, if large the absolute milliseconds per second of progress work). Optimizing the low utilization CPU is always good, but .. I'd lean into that other technique instead (working on it! :D). |
|
Fair question, and you’re mostly right. Some context: this is a prototype running in a moderately realistic test environment. Those numbers come from replaying recorded input as fast as it’ll go, so “1 s commits” means one commit per second of event time, not wall time. The run I quoted is about 22,500 commits in 1.86 s of CPU. Per commit that’s about 83 µs on stock timely: roughly 63 µs in progress tracking and 20 µs for everything else, with only a few records per round. With this PR it’s about 56 µs. So yes, the data plane is nearly empty each round, and this cuts the fixed tax. Run live at one commit a second, it would be close to idle. A heavier program does about 4.7 ms of work per commit and still drops 15% at 1 s commits, falling to 1% at 60 s. Same picture: the tax amortises as rounds fill up. Small rounds at 1 s latency are the regime we expect to run in, so this tax is what we hit. If that’s not a regime you want timely tuned for, fair enough. I’m happy to try the capability-skipping work when it’s ready and see how much of this it recovers. One question about it: here every round advances the input frontier, so every location downstream of it moves. Does skipping regions that hold no capabilities avoid propagating those changes too, or only the operator schedules? |
|
One other bit of context: even though the main engine runs in real time with a low load, having the full-rate replays be as fast as possible is very beneficial, because replays are used for debugging, crash recovery, counterfactual analysis, and sundry other fun purposes where throughput becomes a bottleneck. |
Problem
Tracker::propagate_alldrives a single worklist ordered by(time, location). A location whose frontier moves fromttot'is visited attand again att', and each visit propagates downstream, so the frontier typically steps through an intermediate state (often empty) and every downstream location repeats the work. Pending source and target changes, and the pushed changes, areChangeBatch<(Location, T)>s, so each round also sorts all its changes by location and time. In programs that run many small rounds, this sorting and the repeated visits dominate.Change
propagate_allvisits components in that order. A location outside any cycle is visited once, after every change that can reach it: it applies all its changes to itsimplicationsin oneupdate_iterand pushes the net frontier change directly. The locations of a component with a cycle are visited in order of time, within the component, as before.LocationChanges, aChangeBatch<T, 4>per location), so consolidation sorts a location's few times, never the round's changes by location and time.Tracker::pushednow returns&mut Vec<((Location, T), i64)>: net changes, consolidated per location, in no particular order. This is a public signature change; the in-tree caller (Subgraph) and the module docs are updated todrain(..).Why results and progress semantics are unchanged
A location's
implicationsis aMutableAntichain, which accumulates the same updates to the same counts in any order, so its frontier afterpropagate_allis the same. Because components are visited in topological order, every change that can reach a location outside a cycle has arrived before it is visited, so visiting it once is enough. Within a cyclic component the existing time-ordered argument applies unchanged: changes leaving the component are buffered for later components and cannot return. The pushed changes are consolidated to the net change of each location's frontier, as theChangeBatchmade them before.Subgraphapplies them to each input's frontier, activates the operators whose inputs changed and considers the same shutdowns, and none of these depends on order.output_changes,total_countsandcap_countsare updated exactly as before in step 1. Only the work in between differs. The order of the tracker's logged source and target updates changes, but not their content.Measurements
A new example,
progress_rounds(below, not part of this PR), runs one input through a dataflow for many rounds, one record andadvance_to(round + 1)per round, stepping until the probe passes. One worker, release build, pinned withtaskset,perf staton a c7a.8xlarge, medians of 3 runs:masterchain 100 20000(region of 100 maps)chain 10 100000loop 20 20000(iterate, 20 maps in the body)Run-to-run spread was under 0.05% in instructions and about 2% in time. The "with #827" times come from a second batch, where this PR alone measured 1.48 s, 1.11 s and 1.68 s. The loop gains little because most of its locations share one cycle, which is visited as before.
This came out of FlowLog, a Datalog engine on differential dataflow, where progress tracking was 76% of the samples and sorting
ChangeBatch<(Location, T)>a third of them in a workload committing once a second (279 locations in the root, 29 in one cycle). With this change, #827 and a small change toChangeBatch::compact(not proposed, as it costs instructions on the workloads above), engine CPU fell 32% there (instructions −31%), 15% on another, and about 1% where rounds are rare, with identical outputs.Testing
cargo test --workspace --all-targetsandcargo test --docpass (234 tests), alone and stacked on #827.cargo clippy --workspace --all-targetsadds no warnings (104, down from 108).timely/examples/progress_rounds.rs