Stage 10 — Performance and Optimization

What this stage teaches

Stage 10 is where you stop fixing correctness bugs and start moving numbers. The skill is different: you must measure before and after, attribute cost to a specific frame, and make a change whose benefit you can defend with data. A performance patch without numbers is noise. You learn:

• The three cost buckets that dominate Tez perf work: lock contention (synchronised getters on hot paths), wasted allocation (short-lived objects in tight loops, or copies that could be avoided), and needless work (computing something eagerly that is rarely needed).
• How the real optimisation commits were validated — not always with a benchmark harness, but always with a clear before/after story a committer accepted.
• How to scope a perf patch so it merges: one hotspot, one change, one measurable effect. "While I was here I also…" is how perf PRs die in review.
• Where Tez's hot paths actually are: counters, TaskImpl/TaskAttemptImpl, IFile/TezMerger, and the AM's per-heartbeat task registration.

Patches are 2–300 lines. The smallest, best ones change a handful of lines and cite a profile or a contention analysis.

A word on why this stage comes late in the roadmap: you cannot credibly optimise code you cannot first measure repeatably, and repeatable measurement is exactly the skill Stage 9 forces. A perf patch is a scientific claim — "this change reduces cost X by Y under conditions Z" — and a committer will hold it to a scientific standard. Everything in this stage is downstream of that one idea: no number, no patch.

Prerequisite: Stage 9 (you must be able to write a deterministic, repeatable measurement) plus the deep dives: counters & diagnostics, shuffle & sort, and DAG AppMaster.

Finding Stage 10 issues today

project = TEZ AND resolution = Unresolved
  AND (summary ~ "performance" OR summary ~ "slow" OR summary ~ "optimize"
       OR summary ~ "contention" OR summary ~ "allocation"
       OR summary ~ "bottleneck" OR labels = "performance")
ORDER BY priority DESC, updated DESC

Learn the shapes from history first — a decade of real, measured perf commits:

cd /Users/s0x/src/oss-repos/tez
git log --oneline -i --grep=perf --grep=optimi --grep="speed up"

Heuristic grep for the classic contention shape — synchronized on a getter that a hot loop calls:

grep -rn "synchronized" tez-api/src/main/java/org/apache/tez/common/counters/ | head
grep -rn "new byte\[\|new .*\[\]" \
  tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/sort/impl/IFile.java | head

Case study A — TEZ-2731: kill lock contention on the counter hot path

The canonical "remove synchronized" perf fix. Read it:

git show dc0ee0115   # Fix Tez GenericCounter performance bottleneck

The symptom. Profiling a counter-heavy DAG showed threads piling up on GenericCounter. Every counter increment took a monitor.

The root cause. GenericCounter stored a plain long value and guarded every access with synchronized:

private long value = 0;
public synchronized long getValue()          { return value; }
public synchronized void setValue(long value) { this.value = value; }
public synchronized void increment(long incr) { value += incr; }

Counters are incremented in the innermost loops of every task — key comparisons, bytes written, records read. Under many threads, the monitor on increment is a serialization point: the whole point of a counter (cheap, frequent) is defeated.

The fix — a lock-free atomic. Gopal V replaced the long with an AtomicLong and dropped synchronized entirely:

private final AtomicLong value = new AtomicLong(0);
public long getValue()          { return value.get(); }
public void setValue(long value) { this.value.set(value); }
public void increment(long incr) { value.addAndGet(incr); }

readFields/write were updated to value.set(...) / value.get(). Eleven lines changed, ten removed. No new test — the behaviour is identical; only the contention profile changed. The justification lived in the JIRA: the bottleneck was visible in a profile, and AtomicLong is the textbook fix.

The lesson. A synchronized getter/setter on an object touched by every task thread is a scalability bug. AtomicLong/AtomicReference give you the same memory-visibility guarantee without the monitor. This is the single most common Tez perf patch shape — grep for synchronized on any counter/stat/progress class.

Case study B — TEZ-4250: don't compute counters you won't use

git show 9aeb17b4b   # Optimise TaskImpl::getCounters

The symptom. TaskImpl.getCounters() showed up hot on large DAGs. It was called frequently and did real work every time.

The root cause. getCounters() unconditionally built a fresh TezCounters and copied every counter into it:

TezCounters counters = new TezCounters();
counters.incrAllCounters(this.counters);
...
if (bestAttempt != null) {
  counters.incrAllCounters(bestAttempt.getCounters());
}
return counters;

That allocation-plus-copy happens on every call — but the aggregated copy is only actually needed when speculation is enabled (speculation compares a task's counters across attempts). With speculation off, the caller only needs the best attempt's counters directly, no copy.

The fix — do the expensive path only when needed. Ayush Saxena gated the copy on isSpeculationEnabled() and otherwise returned the best attempt's counters by reference:

TezCounters tezCounters = null;
if (getVertex().isSpeculationEnabled()) {
  tezCounters = new TezCounters();
  tezCounters.incrAllCounters(this.counters);
}
readLock.lock();
try {
  TaskAttempt bestAttempt = selectBestAttempt();
  TezCounters taskCounters = (bestAttempt != null)
      ? bestAttempt.getCounters() : TaskAttemptImpl.EMPTY_COUNTERS;
  if (getVertex().isSpeculationEnabled()) {
    tezCounters.incrAllCounters(taskCounters);
    return tezCounters;
  }
  return taskCounters;
} finally {
  readLock.unlock();
}

Supporting this, setCounters(...) was pushed down to the attempt (selectBestAttempt().setCounters(counters)), and a @VisibleForTesting setCounters was added to the TaskAttempt interface so the behaviour is testable.

The lesson. The fastest work is work you don't do. Before optimising how a computation runs, ask whether it needs to run at all on the common path. Here the common configuration (speculation off) skips an allocation and a full counter copy per call. Look for eager computation guarded by nothing.

Case study C — TEZ-3939: move an expensive string off the hot path

git show f6624c152   # Remove performance hit of precondition check in AM for register running task attempt

The symptom. Registering a running task attempt showed a measurable cost in the AM under high task churn.

The root cause. The precondition check eagerly built its failure message by string concatenation every time, even though the message is only used when the check fails:

Preconditions.checkNotNull(containerInfo,
    "Cannot register task attempt: " + taskSpec.getTaskAttemptID()
        + " to unknown container: " + containerId);

The "..." + taskAttemptID + "..." + containerId string is constructed on every call, allocating and formatting even in the overwhelmingly-common success case.

The fix — lazy message formatting. Jonathan Eagles switched to Guava's format-string overload, which only formats when the check actually fails:

Preconditions.checkNotNull(containerInfo,
    "Cannot register task attempt %s to unknown container %s",
    taskSpec.getTaskAttemptID(), containerId);

Two lines. No test — the behaviour is identical; only the allocation on the success path is gone.

The lesson. Eager message construction inside a Preconditions.checkX(...) or a LOG.debug("..." + x) on a hot path allocates on every call for a message you almost never see. Use the format-string / {} overloads so the cost is paid only on failure. This is the cheapest perf win in the codebase and the easiest to grep for.

Case study D — TEZ-3709: an O(n²) merge that should be O(n)

The most algorithmic Stage 10 fix, and the one that best shows why you measure under scale. Read it:

git show 4d100b2bf   # TezMerger is slow for high number of segments

The symptom. The TezMerger — which merges sorted segments during shuffle — got dramatically slower as the number of segments grew. On a reduce task pulling thousands of map outputs, the merge dominated wall time.

The root cause — ArrayList.remove(0) in a loop. TezMerger built its next merge batch by repeatedly removing the first element:

List<Segment> subList = new ArrayList<Segment>(segments.subList(0, numDescriptors));
// TODO Replace this with a batch operation
for (int i = 0; i < numDescriptors; ++i) {
  segments.remove(0);   // shifts the ENTIRE backing array left, every iteration
}

ArrayList.remove(0) is O(n): it shifts every remaining element down one slot. Do it numDescriptors times and you have O(n²). The // TODO even flagged it — nobody had felt the pain until someone ran enough segments. MergeManager had the same pattern with inMemoryMapOutputs.remove(0) in its in-memory bytes loop.

The fix — bulk removal and unboxed arrays. Jonathan Eagles replaced the shift- per-element loop with a single subList().clear():

// Efficiently bulk remove segments
List<Segment> subList = segments.subList(0, numDescriptors);
List<Segment> subListCopy = new ArrayList<>(subList);
subList.clear();   // one bulk operation, O(n) total
return subListCopy;

and did the same in MergeManager by advancing an offset and calling subList(0, offset).clear() once at the end. He also converted computeBytesInMerges from a List<Long> (boxed, allocating a Long per segment) to a primitive long[], and made the helper static so it doesn't capture the enclosing instance.

The lesson. list.remove(0) inside a loop is an O(n²) trap hiding in plain Java. When you touch any merge/sort/collection path that scales with input size, count the algorithmic complexity, not just the allocations — and measure under scale, because these bugs are invisible at 10 segments and fatal at 10,000. Boxed List<Long> in a hot loop is a second, quieter tax; prefer long[].

Profiling tools: what to reach for

You will not always see the cost in the diff. When you don't, profile:

Profile the AM for scheduler/dispatcher/counter contention (it is the long-running process where contention manifests), and profile a task JVM for IFile/TezMerger/sorter costs. A single fat frame above the noise floor is your target — the three algorithmic case studies above each correspond to exactly one.

The measurement discipline

None of the three fixes above shipped a JMH harness — they shipped a clear cost story a committer accepted. But when a change's benefit is not self-evident from the diff (a synchronized removal and a lazy-format are self-evident; a data- structure change is not), you must bring numbers. The bar:

Methodology:
  - Hardware / JVM named.
  - Workload: OrderedWordCount on MiniTezCluster, or a JMH micro at the call site.
  - Runs: several cold + warm; report median and p95, not a single number.
  - One Hadoop profile, held constant.

Before (<hash>): median X.
After  (this):   median Y.
Evidence: profile/flame graph or contention counts attached.

Two rules that fail review instantly: comparing across different Hadoop profiles, and reporting a single un-warmed run. A profile artifact attached to the JIRA is what turns "looks faster" into "is faster."

When not to optimise. A micro-benchmark win that does not move the end-to-end number is not worth the review cost or the added complexity. Always show the end-to-end impact alongside the micro number: a 2x speedup in IFile.Writer.append that yields a 0.1% DAG-wall-time improvement is usually a no. The corollary — the one that makes committers trust you — is that you are willing to withdraw a patch whose measured benefit turns out to be noise. Perf work is measurement first, code second; the discipline is knowing when the measurement says "don't."

The contribution playbook for this class

1. Find the hotspot with evidence, not intuition — a profile, a contention count, or an obvious per-call allocation the diff makes self-evident.
2. Classify the cost: contention (TEZ-2731), needless work (TEZ-4250), or eager allocation (TEZ-3939). Each has a known fix.
3. Scope to one hotspot. A perf PR that touches three subsystems will not merge.
4. Preserve behaviour exactly. All three case studies are behaviour-preserving; that is why two of them needed no test. If behaviour can change (TEZ-4250's speculation gate), add a @VisibleForTesting hook and a test.
5. Bring numbers when the win isn't self-evident. Attach the profile.
6. Run the affected suites to prove no regression:

mvn -pl tez-api test -Dtest=TestTezCounters -q
mvn -pl tez-dag test -Dtest=TestTaskImpl -q

Common mistakes

Exit criteria — when you're ready for the next stage

• You have shipped one perf patch whose benefit you can defend — either self-evidently behaviour-preserving (a lock removal / lazy format) or backed by before/after numbers and a profile.
• You can classify a hotspot into contention / needless-work / eager-allocation and name the matching case study (TEZ-2731 / TEZ-4250 / TEZ-3939).
• You can explain why two of those three fixes shipped with no test, and when that is not acceptable.
• You have grepped the counter and IFile hot paths for synchronized and per-call allocation and can point at a candidate.

Stage 11 takes you into the compatibility contract.