perf: Avoid unnecessary object copy (#14843)

[MatzeB]

Summary:

This function shows up in internal profiles. This is an AI generated performance optimization (but human reviewed):

Optimized rocksdb::ColumnFamilyOptions::ColumnFamilyOptions by eliminating unnecessary copy construction calls in the hottest callers.

Problem: Strobelight traces (weight 705M+) showed the ColumnFamilyOptions copy constructor being called from GetMinTimeBasedCompactionInterval(cfd->GetLatestCFOptions()) in both TriggerPeriodicCompaction and ComputeTriggerCompactionPeriod. GetLatestCFOptions() calls BuildColumnFamilyOptions which copy-constructs an entire ColumnFamilyOptions object just to read a few scalar fields.

Fix: Changed GetMinTimeBasedCompactionInterval to accept const MutableCFOptions& instead of const ColumnFamilyOptions&. All fields it reads (periodic_compaction_seconds, ttl, compaction_options_fifo.file_temperature_age_thresholds, bottommost_file_compaction_delay) exist in MutableCFOptions. Updated both call sites to pass cfd->GetLatestMutableCFOptions() which returns a const& — no copy needed.

Impact: Eliminates the expensive ColumnFamilyOptions copy construction (which involves copying multiple shared_ptr members with atomic ref-count increments, vectors, and strings) on every periodic compaction check and trigger computation period calculation, for every column family in the loop.

Differential Revision: D107676627

[meta-codesync]

@MatzeB has exported this pull request. If you are a Meta employee, you can view the originating Diff in D107676627.

[github-actions]

✅ clang-tidy: No findings on changed lines

Completed in 267.3s.

[github-actions]

🟡 Codex Code Review

Auto-triggered after CI passed — reviewing commit 8396445

---

❌ Codex review failed before producing findings.

WARNING: proceeding, even though we could not create PATH aliases: Refusing to create helper binaries under temporary dir "/tmp" (codex_home: AbsolutePathBuf("/tmp/codex-home"))
error: the argument '--base <BRANCH>' cannot be used with '[PROMPT]'

Usage: codex exec review --commit <SHA> --base <BRANCH> --title <TITLE> --model <MODEL> --config <key=value> --dangerously-bypass-approvals-and-sandbox --output-last-message <FILE> [PROMPT]

For more information, try '--help'.

---

ℹ️ About this response

Generated by Codex CLI.
Review methodology: claude_md/code_review.md

Limitations:

• Codex may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /codex-review [context] — Request a code review
• /codex-query <question> — Ask about the PR or codebase

[github-actions]

✅ Claude Code Review

Auto-triggered after CI passed — reviewing commit 8396445

---

Summary

Clean performance optimization that eliminates unnecessary ColumnFamilyOptions copy construction in two hot call sites by switching the file-static GetMinTimeBasedCompactionInterval to accept const MutableCFOptions& instead. All four accessed fields exist in MutableCFOptions with identical types. No behavioral, API, or thread-safety concerns.

High-severity findings (0):

No high-severity findings.

Full review (click to expand)

Findings

🔴 HIGH

None.

🟡 MEDIUM

None.

🟢 LOW / NIT

None.

Cross-Component Analysis

Context	Does code execute?	Assumptions hold?	Action needed?
WritePreparedTxnDB	YES (via base DBImpl)	YES	Safe
ReadOnly DB	NO (no periodic task scheduler)	N/A	Safe
SecondaryInstance	NO (no periodic task scheduler)	N/A	Safe
User-defined timestamps	YES	YES (fields unrelated to UDT)	Safe
FIFO compaction	YES	YES (fifo thresholds field present in MutableCFOptions)	Safe
Universal compaction	YES	YES	Safe

Field verification (MutableCFOptions vs ColumnFamilyOptions):

Field	Type in CFOptions	Type in MutableCFOptions	Match?
periodic_compaction_seconds	uint64_t	uint64_t	YES
ttl	uint64_t	uint64_t	YES
compaction_options_fifo	CompactionOptionsFIFO	CompactionOptionsFIFO	YES
bottommost_file_compaction_delay	uint32_t	uint32_t	YES

Reference safety: GetLatestMutableCFOptions() returns const MutableCFOptions& to a member variable. Both call sites hold the DB mutex as required. The reference is consumed synchronously within the static function -- no dangling reference risk.

Caller scope: GetMinTimeBasedCompactionInterval is static (file-local), called from exactly two places in db_impl.cc. No external callers exist.

Positive Observations

• Well-targeted optimization backed by profiling data (Strobelight, weight 705M+)
• Eliminates copy construction of ColumnFamilyOptions which includes 12+ shared_ptr atomic operations, multiple vector copies, and string copies
• Zero behavioral change -- same fields, same types, same values, same logic
• No API impact (file-static function)
• Existing test coverage in periodic_task_scheduler_test.cc is sufficient since no behavior changed

---

ℹ️ About this response

Generated by Claude Code.
Review methodology: claude_md/code_review.md

Limitations:

• Claude may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /claude-review [context] — Request a code review
• /claude-query <question> — Ask about the PR or codebase