Commit 7e2877a3 authored by Maged Michael's avatar Maged Michael Committed by Facebook GitHub Bot

RequestContext: Optimize copying

Summary:
Reduce the cost of copying by keeping a vector of cleared RequestData references instead of keeping a hash map of the state of all RequestData references.

Also, added FOLLY_ALWAYS_INLINE to functions called in copying of request contexts.

Reviewed By: yfeldblum

Differential Revision: D20863318

fbshipit-source-id: 29cc38ac6e8abde3acaffcf8fa4d1e7ccbd45e87
parent a2c4f8cf
...@@ -74,6 +74,7 @@ Synchronized<F14FastMap<std::string, uint32_t>>& RequestToken::getCache() { ...@@ -74,6 +74,7 @@ Synchronized<F14FastMap<std::string, uint32_t>>& RequestToken::getCache() {
return *cache; return *cache;
} }
FOLLY_ALWAYS_INLINE
void RequestData::acquireRef() { void RequestData::acquireRef() {
auto rc = keepAliveCounter_.fetch_add( auto rc = keepAliveCounter_.fetch_add(
kClearCount + kDeleteCount, std::memory_order_relaxed); kClearCount + kDeleteCount, std::memory_order_relaxed);
...@@ -100,6 +101,7 @@ void RequestData::releaseRefDeleteOnly() { ...@@ -100,6 +101,7 @@ void RequestData::releaseRefDeleteOnly() {
} }
} }
FOLLY_ALWAYS_INLINE
void RequestData::releaseRefClearDelete() { void RequestData::releaseRefClearDelete() {
auto rc = keepAliveCounter_.fetch_sub( auto rc = keepAliveCounter_.fetch_sub(
kClearCount + kDeleteCount, std::memory_order_acq_rel) - kClearCount + kDeleteCount, std::memory_order_acq_rel) -
...@@ -148,12 +150,8 @@ struct RequestContext::StateHazptr::Combined : hazptr_obj_base<Combined> { ...@@ -148,12 +150,8 @@ struct RequestContext::StateHazptr::Combined : hazptr_obj_base<Combined> {
SingleWriterFixedHashMap<RequestToken, RequestData*> requestData_; SingleWriterFixedHashMap<RequestToken, RequestData*> requestData_;
// This must be optimized for iteration, its hot path is setContext // This must be optimized for iteration, its hot path is setContext
SingleWriterFixedHashMap<RequestData*, bool> callbackData_; SingleWriterFixedHashMap<RequestData*, bool> callbackData_;
// Hash map to keep track of Clear and Delete counts. The presence // Vector of cleared data. Accessed only sequentially by writers.
// of a key indicates holding a Delete count for the request data std::vector<std::pair<RequestToken, RequestData*>> cleared_;
// (i.e., delete when the Delete count goes to zero). A value of
// true indicates holding a Clear counts for the request data (i.e.,
// call onClear() when the Clear count goes to zero).
F14FastMap<RequestData*, bool> refs_;
Combined() Combined()
: requestData_(kInitialCapacity), callbackData_(kInitialCapacity) {} : requestData_(kInitialCapacity), callbackData_(kInitialCapacity) {}
...@@ -178,7 +176,6 @@ struct RequestContext::StateHazptr::Combined : hazptr_obj_base<Combined> { ...@@ -178,7 +176,6 @@ struct RequestContext::StateHazptr::Combined : hazptr_obj_base<Combined> {
for (auto it = requestData_.begin(); it != requestData_.end(); ++it) { for (auto it = requestData_.begin(); it != requestData_.end(); ++it) {
auto p = it.value(); auto p = it.value();
if (p) { if (p) {
refs_.insert({p, true});
p->acquireRef(); p->acquireRef();
} }
} }
...@@ -186,11 +183,16 @@ struct RequestContext::StateHazptr::Combined : hazptr_obj_base<Combined> { ...@@ -186,11 +183,16 @@ struct RequestContext::StateHazptr::Combined : hazptr_obj_base<Combined> {
/* releaseDataRefs - Called only once from ~Combined */ /* releaseDataRefs - Called only once from ~Combined */
void releaseDataRefs() { void releaseDataRefs() {
for (auto pair : refs_) { if (!cleared_.empty()) {
if (pair.second) { for (auto& pair : cleared_) {
pair.first->releaseRefClearDelete(); pair.second->releaseRefDeleteOnly();
} else { requestData_.erase(pair.first);
pair.first->releaseRefDeleteOnly(); }
}
for (auto it = requestData_.begin(); it != requestData_.end(); ++it) {
RequestData* data = it.value();
if (data) {
data->releaseRefClearDelete();
} }
} }
} }
...@@ -215,6 +217,7 @@ struct RequestContext::StateHazptr::Combined : hazptr_obj_base<Combined> { ...@@ -215,6 +217,7 @@ struct RequestContext::StateHazptr::Combined : hazptr_obj_base<Combined> {
RequestContext::StateHazptr::StateHazptr() = default; RequestContext::StateHazptr::StateHazptr() = default;
FOLLY_ALWAYS_INLINE
RequestContext::StateHazptr::StateHazptr(const StateHazptr& o) { RequestContext::StateHazptr::StateHazptr(const StateHazptr& o) {
Combined* oc = o.combined(); Combined* oc = o.combined();
if (oc) { if (oc) {
...@@ -238,6 +241,7 @@ RequestContext::StateHazptr::Combined* RequestContext::StateHazptr::combined() ...@@ -238,6 +241,7 @@ RequestContext::StateHazptr::Combined* RequestContext::StateHazptr::combined()
return combined_.load(std::memory_order_acquire); return combined_.load(std::memory_order_acquire);
} }
FOLLY_ALWAYS_INLINE
RequestContext::StateHazptr::Combined* RequestContext::StateHazptr::Combined*
RequestContext::StateHazptr::ensureCombined() { RequestContext::StateHazptr::ensureCombined() {
auto c = combined(); auto c = combined();
...@@ -248,11 +252,13 @@ RequestContext::StateHazptr::ensureCombined() { ...@@ -248,11 +252,13 @@ RequestContext::StateHazptr::ensureCombined() {
return c; return c;
} }
FOLLY_ALWAYS_INLINE
void RequestContext::StateHazptr::setCombined(Combined* p) { void RequestContext::StateHazptr::setCombined(Combined* p) {
p->set_cohort_tag(&cohort_); p->set_cohort_tag(&cohort_);
combined_.store(p, std::memory_order_release); combined_.store(p, std::memory_order_release);
} }
FOLLY_ALWAYS_INLINE
bool RequestContext::StateHazptr::doSetContextData( bool RequestContext::StateHazptr::doSetContextData(
const RequestToken& token, const RequestToken& token,
std::unique_ptr<RequestData>& data, std::unique_ptr<RequestData>& data,
...@@ -276,6 +282,7 @@ bool RequestContext::StateHazptr::doSetContextData( ...@@ -276,6 +282,7 @@ bool RequestContext::StateHazptr::doSetContextData(
return result.changed; return result.changed;
} }
FOLLY_ALWAYS_INLINE
RequestContext::StateHazptr::SetContextDataResult RequestContext::StateHazptr::SetContextDataResult
RequestContext::StateHazptr::doSetContextDataHelper( RequestContext::StateHazptr::doSetContextDataHelper(
const RequestToken& token, const RequestToken& token,
...@@ -333,6 +340,7 @@ RequestContext::StateHazptr::doSetContextDataHelper( ...@@ -333,6 +340,7 @@ RequestContext::StateHazptr::doSetContextDataHelper(
replaced}; replaced};
} }
FOLLY_ALWAYS_INLINE
RequestContext::StateHazptr::Combined* FOLLY_NULLABLE RequestContext::StateHazptr::Combined* FOLLY_NULLABLE
RequestContext::StateHazptr::eraseOldData( RequestContext::StateHazptr::eraseOldData(
RequestContext::StateHazptr::Combined* cur, RequestContext::StateHazptr::Combined* cur,
...@@ -350,7 +358,6 @@ RequestContext::StateHazptr::eraseOldData( ...@@ -350,7 +358,6 @@ RequestContext::StateHazptr::eraseOldData(
// If the caller guarantees thread-safety, then erase the // If the caller guarantees thread-safety, then erase the
// entry in the current version. // entry in the current version.
cur->requestData_.erase(token); cur->requestData_.erase(token);
cur->refs_.erase(olddata);
olddata->releaseRefClearDelete(); olddata->releaseRefClearDelete();
} else { } else {
// If there may be concurrent readers, then copy-on-erase. // If there may be concurrent readers, then copy-on-erase.
...@@ -363,6 +370,7 @@ RequestContext::StateHazptr::eraseOldData( ...@@ -363,6 +370,7 @@ RequestContext::StateHazptr::eraseOldData(
return newCombined; return newCombined;
} }
FOLLY_ALWAYS_INLINE
RequestContext::StateHazptr::Combined* FOLLY_NULLABLE RequestContext::StateHazptr::Combined* FOLLY_NULLABLE
RequestContext::StateHazptr::insertNewData( RequestContext::StateHazptr::insertNewData(
RequestContext::StateHazptr::Combined* cur, RequestContext::StateHazptr::Combined* cur,
...@@ -383,7 +391,6 @@ RequestContext::StateHazptr::insertNewData( ...@@ -383,7 +391,6 @@ RequestContext::StateHazptr::insertNewData(
data->onSet(); data->onSet();
} }
if (data) { if (data) {
cur->refs_.insert({data.get(), true});
data->acquireRef(); data->acquireRef();
} }
cur->requestData_.insert(token, data.release()); cur->requestData_.insert(token, data.release());
...@@ -482,8 +489,8 @@ void RequestContext::StateHazptr::clearContextData(const RequestToken& token) { ...@@ -482,8 +489,8 @@ void RequestContext::StateHazptr::clearContextData(const RequestToken& token) {
} // Unlock mutex_ } // Unlock mutex_
DCHECK(data); DCHECK(data);
data->releaseRefClearOnly(); data->releaseRefClearOnly();
replaced->refs_[data] = false; // Clear reference already released
DCHECK(replaced); DCHECK(replaced);
replaced->cleared_.emplace_back(std::make_pair(token, data));
replaced->retire(); replaced->retire();
} }
......
...@@ -300,8 +300,8 @@ onSet 12 ns 12 ns 0 ns 12 ns ...@@ -300,8 +300,8 @@ onSet 12 ns 12 ns 0 ns 12 ns
onUnset 12 ns 12 ns 0 ns 12 ns onUnset 12 ns 12 ns 0 ns 12 ns
setContext 46 ns 44 ns 1 ns 42 ns setContext 46 ns 44 ns 1 ns 42 ns
RequestContextScopeGuard 113 ns 103 ns 3 ns 101 ns RequestContextScopeGuard 113 ns 103 ns 3 ns 101 ns
ShallowCopyRequestC...-replace 287 ns 279 ns 5 ns 270 ns ShallowCopyRequestC...-replace 230 ns 221 ns 4 ns 217 ns
ShallowCopyReq...-keep&replace 1191 ns 1149 ns 21 ns 1121 ns ShallowCopyReq...-keep&replace 904 ns 893 ns 5 ns 886 ns
============================== 10 threads ============================== ============================== 10 threads ==============================
hasContextData 1 ns 1 ns 0 ns 1 ns hasContextData 1 ns 1 ns 0 ns 1 ns
getContextData 2 ns 1 ns 0 ns 1 ns getContextData 2 ns 1 ns 0 ns 1 ns
...@@ -309,7 +309,7 @@ onSet 2 ns 2 ns 0 ns 1 ns ...@@ -309,7 +309,7 @@ onSet 2 ns 2 ns 0 ns 1 ns
onUnset 2 ns 2 ns 0 ns 1 ns onUnset 2 ns 2 ns 0 ns 1 ns
setContext 11 ns 7 ns 2 ns 5 ns setContext 11 ns 7 ns 2 ns 5 ns
RequestContextScopeGuard 22 ns 15 ns 5 ns 11 ns RequestContextScopeGuard 22 ns 15 ns 5 ns 11 ns
ShallowCopyRequestC...-replace 61 ns 43 ns 14 ns 30 ns ShallowCopyRequestC...-replace 51 ns 32 ns 11 ns 24 ns
ShallowCopyReq...-keep&replace 123 ns 121 ns 0 ns 120 ns ShallowCopyReq...-keep&replace 102 ns 98 ns 2 ns 96 ns
======================================================================== ========================================================================
*/ */
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment