Mf hot path optimize 1#725
Draft
crimson11 wants to merge 14 commits into
Draft
Conversation
New benchmark for SkeletonEvent::Allocate and SkeletonEvent::Send added.
Repaced lock with std::atomic access. This is the "smarter approach"! The lock was overdone and didn't add any value. Issue: SWP-254209
GetNewSamples() is a hot path. Previously every call resolved the typed event binding via GetTypedEventBinding(), which performed a nullptr check, a dynamic_cast and a downcast assertion on each invocation. Compute the typed binding once during construction and cache it in the new typed_event_binding_ member. The cached pointer stays valid across moves since the defaulted move only transfers ownership of the underlying heap object without changing its address.
GetNewSamples() calls TraceGetNewSamples() unconditionally on every invocation. The function is too large to inline, so each call incurred a call/jump even when tracing for this trace point is disabled. Split it into a small inlineable stub (TraceGetNewSamples) that only checks enable_get_new_samples and dispatches to the out-of-line TraceGetNewSamplesTracePoint() when tracing is actually enabled. In the common (disabled) case this collapses to a single bool check with no call.
The EventSlotStatus accessors (GetTimeStamp, IsTimeStampBetween, etc.) are trivial bit-manipulation operations invoked per slot inside the GetNewSamples() sample-collection loop, but were defined out-of-line in the .cpp, incurring a function call each. Move all accessors and constructors into the header as inline definitions so they can be inlined into the hot loop.
GetSubscriptionState() and GetNewSamplesSlotIndices() are part of the GetNewSamples()/GetNumNewSamplesAvailable() call chain but were defined out-of-line. Move them into the header as inline definitions. To fully inline GetSubscriptionState(), also move the SubscriptionStateMachineStateToSubscriptionState() converter inline into subscription_helpers.h.
ReferenceTransactionBegin/Commit/Abort are on the GetNewSamples() call chain (invoked from ConsumerEventDataControlLocalView::ReferenceNextEvent() for every candidate slot) but were defined out-of-line. Inline them into the header with a fast path: the uncontended case (the Transaction-END bit is not set) requires no function call. The rare contention-handling path is delegated to the out-of-line WaitForTransactionEndToBecomeFalse() helper, keeping the inlined body small.
This is a non-inlined call on the hot-path. Commented out - should be conditionally done only, if performance measurement is required.
Allocate() and both Send() overloads are hot paths. Previously every call resolved the typed event binding via GetTypedEventBinding(), which performed a dynamic_cast and a downcast assertion on each invocation. Compute the typed binding once during construction and cache it in the new typed_event_binding_ member. The cached pointer stays valid across moves since the defaulted move only transfers ownership of the underlying heap object without changing its address. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ation TryAllocateSlot() and AllocateNextSlot() lie on the SkeletonEvent::Allocate() hot path but were defined in the .cpp, preventing inlining across translation units. Move both definitions into the header as inline template functions so that the compare-exchange in TryAllocateSlot() and the bounded retry loop in AllocateNextSlot() can be inlined into their callers. The explicit template instantiations remain in the .cpp. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
AllocateNextSlot() is called by SkeletonEventCommon::AllocateSlot() on the hot path but was defined in the .cpp, preventing inlining across translation units. Move AllocateNextSlot() into the header as an inline template function, along with the trivial GetAsilBEventDataControlLocal() accessor which is queried on the same path. Together with the inlined ProviderEventDataControlLocalView slot allocation this yields a fully inlinable allocation chain down to the compare-exchange. The explicit template instantiations remain in the .cpp. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…llback SkeletonEvent::Send(SampleAllocateePtr) calls CreateTracingSendWithAllocateCallback() on every invocation. The previous single function contained the callback construction, so even the common (tracing disabled) case could not collapse to a cheap check. Split it into a small inlineable stub (CreateTracingSendWithAllocateCallback) that only checks enable_send_with_allocate and dispatches to the out-of-line CreateSendWithAllocateTraceCallbackHandler() when tracing is actually enabled. In the common (disabled) case this collapses to a single bool check returning an empty optional, with no callback construction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
EventReady() lies on the SkeletonEvent::Send() hot path but was defined in the .cpp, preventing inlining across translation units. Move it into the header as an inline template function so that the single atomic store can be inlined into its callers. The explicit template instantiations remain in the .cpp. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
EventReady() is called by SkeletonEventCommon::Send() on the hot path but was defined in the .cpp, preventing inlining across translation units. Move it into the header as an inline template function. Together with the inlined ProviderEventDataControlLocalView::EventReady() this yields a fully inlinable event-ready path down to the atomic store. The explicit template instantiations remain in the .cpp. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
crimson11
force-pushed
the
mf_hot_path_optimize_1
branch
from
July 18, 2026 21:53
056236a to
de649ed
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Optimizations for our two hottest HOT-PATHS: