This is an automated email from the ASF dual-hosted git repository.
pitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 9a18992678 GH-47774: [C++] Force-inline Status::~Status() to improve
reading performance (#50749)
9a18992678 is described below
commit 9a18992678f86f140c6c76f427678031aee7d493
Author: Zhou Hongfeng <[email protected]>
AuthorDate: Tue Oct 27 20:43:33 2026 +0800
GH-47774: [C++] Force-inline Status::~Status() to improve reading
performance (#50749)
### Rationale for this change
`Status` is returned by nearly every API on hot paths (e.g. IPC/Parquet
reading), so its destructor runs extremely frequently. The destructor body is
already optimized for the common success case: it only checks `state_ != NULL`
(with `ARROW_PREDICT_FALSE`) and returns immediately for an OK status,
delegating the rare error cleanup to the out-of-line `DeleteState()`.
However, without an explicit inlining hint, some compilers/optimization
levels may not inline this destructor, turning the trivial `state_ == NULL`
fast path into a real function call at every call site. Forcing it inline
removes this call overhead on hot read paths and improves read throughput.
In our perf test (reads a parquet file of 6.7GB), `Status::~Status` cost
about 20 seconds, as the following flamegraph shows:
<img width="1381" height="254" alt="image"
src="https://github.com/user-attachments/assets/c62601b8-6d1f-455a-9ee5-9edc4ea57852";
/>
After inlining `Status::~Status`, the hotspot disappeared. End-to-end
latency was also reduced by 20 seconds, as expected.
<img width="1367" height="263" alt="image"
src="https://github.com/user-attachments/assets/78ec8f84-f9b7-4d74-b767-9046c1d40eaf";
/>
### What changes are included in this PR?
- Annotate `Status::~Status()` in `cpp/src/arrow/status.h` with
`ARROW_FORCE_INLINE` (`__attribute__((always_inline)) inline` on GCC/Clang,
`__forceinline` on MSVC), so the cheap null-check fast path is always inlined
while the cold error path still goes through the non-inlined `DeleteState()`.
### Are these changes tested?
Yes, by existing tests. This change does not alter any behavior of
`Status`; the existing `Status` unit tests and the full C++ test suite cover
it. The performance improvement can be observed with the existing read-path
benchmarks.
### Are there any user-facing changes?
No. This is an internal performance optimization with no API or behavior
change.
* GitHub Issue: #47774
Lead-authored-by: Antoine Pitrou <[email protected]>
Co-authored-by: zhouhongfeng <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/src/arrow/status.cc | 4 +---
cpp/src/arrow/status.h | 17 +++++++++--------
2 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/cpp/src/arrow/status.cc b/cpp/src/arrow/status.cc
index 50eb001c3a..5e54c267fb 100644
--- a/cpp/src/arrow/status.cc
+++ b/cpp/src/arrow/status.cc
@@ -32,9 +32,7 @@ Status::Status(StatusCode code, std::string msg,
std::shared_ptr<StatusDetail> d
void Status::CopyFrom(const Status& s) {
if (ARROW_PREDICT_FALSE(state_ != NULL)) {
- if (!state_->is_constant) {
- DeleteState();
- }
+ DeleteState();
}
if (s.state_ == nullptr) {
state_ = nullptr;
diff --git a/cpp/src/arrow/status.h b/cpp/src/arrow/status.h
index 5757440cee..39f31141e8 100644
--- a/cpp/src/arrow/status.h
+++ b/cpp/src/arrow/status.h
@@ -148,11 +148,12 @@ class ARROW_EXPORT [[nodiscard]] Status : public
util::EqualityComparable<Status
public:
// Create a success status.
constexpr Status() noexcept : state_(NULLPTR) {}
- ~Status() noexcept {
+
+ // GH-47774: force-inline the Status destructor's hot path
+ // and keep it as lean as possible.
+ ARROW_FORCE_INLINE ~Status() noexcept {
if (ARROW_PREDICT_FALSE(state_ != NULL)) {
- if (!state_->is_constant) {
- DeleteState();
- }
+ DeleteState();
}
}
@@ -409,7 +410,9 @@ class ARROW_EXPORT [[nodiscard]] Status : public
util::EqualityComparable<Status
void DeleteState() noexcept {
// ARROW-2400: On certain compilers, splitting off the slow path improves
// performance significantly.
- delete state_;
+ if (!state_->is_constant) {
+ delete state_;
+ }
}
void CopyFrom(const Status& s);
inline void MoveFrom(Status& s);
@@ -419,9 +422,7 @@ class ARROW_EXPORT [[nodiscard]] Status : public
util::EqualityComparable<Status
void Status::MoveFrom(Status& s) {
if (ARROW_PREDICT_FALSE(state_ != NULL)) {
- if (!state_->is_constant) {
- DeleteState();
- }
+ DeleteState();
}
state_ = s.state_;
s.state_ = NULLPTR;
