felipecrv commented on code in PR #44493:
URL: https://github.com/apache/arrow/pull/44493#discussion_r1815090479
##########
cpp/src/arrow/status.h:
##########
@@ -363,17 +357,25 @@ class ARROW_EXPORT [[nodiscard]] Status : public
util::EqualityComparable<Status
void AddContextLine(const char* filename, int line, const char* expr);
#endif
+ // Construct a Status which is not destroyed and is cheap to copy.
+ static Status MakeStatic(StatusCode code, std::string msg,
+ std::shared_ptr<StatusDetail> detail);
private:
struct State {
StatusCode code;
std::string msg;
std::shared_ptr<StatusDetail> detail;
+ bool is_static = false;
};
// OK status has a `NULL` state_. Otherwise, `state_` points to
// a `State` structure containing the error code and message(s)
State* state_;
void DeleteState() {
+ // ARROW-2400: On certain compilers, splitting off the slow path improves
+ // performance significantly.
+ if (ARROW_PREDICT_TRUE(state_ == NULL)) return;
+ if (ARROW_PREDICT_FALSE(state_->is_static)) return;
Review Comment:
I'm saying it would be better to the compiler as well not only humans. What
the previous code achieved with the `ARROW_PREDICT_FALSE` was telling the
compiler that `DeleteState()` is code and shouldn't be inlined. Since lots of
`Status` are destroyed all over the place, that reduces binary size and avoids
pollution of the I-cache with cold code.
```cpp
if (ARROW_PREDICT_FALSE(state_ != NULL)) {
DeleteState();
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
