mirror of
https://github.com/facebook/rocksdb.git
synced 2026-11-07 11:32:34 -05:00
Josh Kang
e591f5f1ab
Summary: Pull Request resolved: https://github.com/meta/rocksdb/pull/15608 ## Summary RocksDB's `.clang-tidy` enables a set of performance checks that flag needless copies: - `performance-unnecessary-value-param` - `performance-unnecessary-copy-initialization` - `performance-no-automatic-move` - `performance-move-const-arg` - `performance-for-range-copy` - `cppcoreguidelines-missing-std-forward` A full scan reports 372 findings from these checks in existing code. Most are `std::string`, `Options`, `std::vector` or `std::function` parameters that are copied on every call but only read, and copies that can't be moved. This diff fixes 350 of them, plus one that upstream has since added; upstream has since fixed ten more. What changed: - By-value parameters that are only read are now `const T&`. Declaration and definition were updated together, and default arguments are unchanged. - By-value "sink" parameters that are stored are `std::move`d at their last use, with `<utility>` added where needed. Callers that forwarded into a sink overload now move. For example `VersionEditHandlerPointInTime` now calls the by-value base constructor, so the column family list is moved instead of copied during manifest recovery. - `const auto x = <ref expr>` locals that are only read became `const auto&`, except where the copy is intentional. Locals returned from functions are no longer `const`, so they can be moved. Range-for loops bind `const auto&`. No-op `std::move` calls (on const objects, or into `const&` parameters) are removed. - Forwarding-reference callables are `std::forward`ed at their single call. - The C API change is made in `tools/c_api_gen/c_base.cc` and regenerated into `db/c.cc`. Compatibility: virtual function signatures are unchanged everywhere, including `FileSystem::ReadAsync`, `EventListener` callbacks and `Iterator::GetProperty`. Three non-virtual public functions now take `const&` instead of by-value: `LDBTool::Run(const Options&)`, `NewSstFileManager(..., const std::string& trash_dir, ...)` and `MemoryUtil::GetApproximateMemoryUsageByType` (its `cache_set` parameter). All stay source compatible for callers. RocksDB doesn't guarantee ABI stability. Copies made on purpose were kept: snapshots taken under a mutex in `EventHelpers`, copy-constructor tests, and a callback whose type must match `void (*)(T)`. Reviewed By: xingbowang Differential Revision: D123186313 fbshipit-source-id: 519725966c349167fa859c822892333f1b464139