[damccorm]

Note: this PR was auto-generated by tooling. Before moving it out of draft, the author has done a full review of the PR, specifically evaluating it on the following criteria:

• Is the description accurate? In particular, are the benchmarks correct, meaningful, and reproducible.
• Does the PR avoid doing harm (changes should be non-functional, perf only)?
• (Judgement) Is the code change (and cost of reviewing) worth the performance improvement?

Full details of the methodology of generation and review can be found here - Apache Beam Python SDK Performance PRs Methodology & Branches Impact Ranking

If you have concerns with the PR or with the underlying methodology used to generate this, please comment below - they will be addressed by the author not the tooling :)

Summary of changes:

Optimizes MetricsContainer._get_typed_metric_name (get_counter, get_distribution, get_gauge, get_string_set, get_bounded_trie) in sdks/python/apache_beam/metrics/execution.py:

1. Cache _TypedMetricName(cell_type, metric_name) on metric_name._typed_metric_names instead of allocating a new _TypedMetricName wrapper object on every metric cell lookup.

Dataflow benchmarks showing improvement:

• Before the change: 2026-11-12_13_27_21-10243732593640248076
• After the change: 2026-11-12_13_27_21-17180255097007214814
• Summary of perf improvement: In a single-worker (n1-standard-8, --autoscaling_algorithm=NONE --num_workers=1 --max_num_workers=1) Cloud Dataflow pipeline running 40,000,000 elements (240,000,000 metric lookups) · 2× ParDo(MetricsContainer.get_counter), built-in transform stage execution time improved from 2,297,401 ms -> 1,543,987 ms (1.49x faster, -32.8%) (total single-worker job vCPU time: 3353s -> 2611s).

Additional benchmarks

• Strict Single-PR Isolated Microbenchmark (upstream/master vs upstream/master + this PR alone): bench_31_metrics_container_typed_name_cache (c2feb961eef) (on branch perf/py-benchmarks-suite) — 640,000 MetricsContainer.get_counter(mn).inc() calls improved from 377.38 ms -> 140.37 ms (2.69x faster, -62.8%, -237.01 ms) in strict isolation.
• On-Worker Cloud Dataflow n2-standard-4 Benchmark (p28_ms:13_metrics_container_typed_name_640k & subbench_ms:03_metrics_container_lookup_ms): On a Cloud Dataflow n2-standard-4 worker, 640,000 MetricsContainer.get_counter(mn) lookups improved from 567 ms -> 216 ms (2.63x faster, -61.9%), and 512,000 get_counter(MetricName).inc() updates in WorkerSubsystemBenchDoFn improved from 517 ms -> 180 ms (2.87x faster).
• End-to-End Cloud Dataflow Benchmark (core_window_row_combine & s8-loadtest): Included in run_core_window_row_combine_pipeline (c2feb961eef) (3,000,000 generated events -> 9,000,000 sliding-windowed elements on n2-standard-4), which improved end-to-end worker stage execution time by 1.80x (-44.5%, 599.6s -> 332.9s).

Testing

• Verified with pytest sdks/python/apache_beam/metrics/execution_test.py.

[damccorm]

/gemini review

[damccorm]

/gemini review

[codecov]

Codecov Report

❌ Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 57.64%. Comparing base (d3078c1) to head (d29f414).
⚠️ Report is 111 commits behind head on master.

Files with missing lines	Patch %	Lines
sdks/python/apache_beam/metrics/execution.py	94.11%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #41275      +/-   ##
============================================
+ Coverage     57.48%   57.64%   +0.15%     
  Complexity     2492     2492              
============================================
  Files          1204     1198       -6     
  Lines        171265   171357      +92     
  Branches       2103     2103              
============================================
+ Hits          98456    98782     +326     
+ Misses        70157    69923     -234     
  Partials       2652     2652              

Flag	Coverage Δ
python	80.51% <94.11%> (+0.30%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[damccorm]

/gemini review

[github-actions]

📋 Review Summary

This PR optimizes MetricsContainer cell lookups (get_counter, get_distribution, get_gauge, get_string_set, get_bounded_trie) by caching _TypedMetricName instances per cell_type on metric_name._typed_metric_names. While this avoids repeated _TypedMetricName allocations and MetricName.fast_name() computations on hot paths, storing _TypedMetricName back on MetricName.__dict__ creates a reference cycle (MetricName -> _typed_metric_names -> _TypedMetricName -> MetricName) that breaks pickling and deep-copying of MetricName instances after lookup.

🔍 General Feedback

• Unpickling / deepcopy failure due to cyclic __reduce__ reconstruction: _TypedMetricName.__reduce__ reconstructs via _TypedMetricName(self.cell_type, self.metric_name), and _TypedMetricName.__init__ immediately calls metric_name.fast_name(). When a MetricName with _typed_metric_names in its __dict__ is unpickled or passed to copy.deepcopy, _TypedMetricName(cell_type, mn) is invoked while mn.__dict__ is still being built (before the BUILD step populates mn.name, mn.namespace, etc.), raising AttributeError: 'MetricName' object has no attribute 'name'.
• Recommended fix & test coverage: Define __reduce__ (or __getstate__/__setstate__) on MetricName in sdks/python/apache_beam/metrics/metricbase.py so transient _typed_metric_names caches are excluded from serialization, and add a unit test in execution_test.py verifying that pickle.loads(pickle.dumps(metric_name)) and copy.deepcopy(metric_name) succeed after MetricsContainer.get_counter(metric_name).

---

💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.

[github-actions]

🟠 High: Caching _TypedMetricName(cell_type, metric_name) in metric_name._typed_metric_names introduces a reference cycle (metric_name -> _typed_metric_names -> _TypedMetricName -> metric_name) that causes pickle.loads(pickle.dumps(metric_name)), dill.loads(dill.dumps(metric_name)), and copy.deepcopy(metric_name) (as well as MetricKey / MetricResult containing metric_name) to fail with AttributeError: 'MetricName' object has no attribute 'name' once metric_name has been passed to MetricsContainer.get_*.

During unpickling or deepcopy of metric_name, Python allocates an uninitialized MetricName.__new__(MetricName) and memoizes it before reconstructing its __dict__. Reconstructing _typed_metric_names invokes _TypedMetricName.__reduce__() -> _TypedMetricName(cell_type, metric_name), whose __init__ calls metric_name.fast_name() before metric_name.__dict__ has been populated.

To fix this, define __reduce__ on MetricName in sdks/python/apache_beam/metrics/metricbase.py so the transient cache is not serialized:

def __reduce__(self):
  return MetricName, (self.namespace, self.name, self.urn, self.labels)

[damccorm]

Great catch! Fixed in d29f414: updated _TypedMetricName.__reduce__ and __init__ to pass fast_name directly so unpickling / pickler / deepcopy of a MetricName cycle does not invoke metric_name.fast_name() before metric_name.__dict__ is restored, and added TestMetricsContainer.test_pickle_and_deepcopy_metric_name_after_caching.

[damccorm]

/gemini review

[github-actions]

📋 Review Summary

This PR optimizes metric lookups in MetricsContainer (get_counter, get_distribution, get_gauge, get_string_set, get_bounded_trie) by caching _TypedMetricName instances per cell_type on MetricName. It also updates _TypedMetricName.__reduce__ and __init__ to accept fast_name directly so that MetricName instances containing cached _TypedMetricName references remain safely serializable and copyable.

🔍 General Feedback

• The caching mechanism is clean, safe, and avoids redundant _TypedMetricName allocations, MetricName.fast_name() string formatting, and hash computations on hot metric update paths.
• The previous serialization cycle issue during pickle/dill/deepcopy has been properly resolved and covered with unit tests in execution_test.py.

---

💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.