Draft
Conversation
bemerybmw
force-pushed
the
brem_move_transaction_log_set
branch
2 times, most recently
from
12097ad to
e8a8109
Compare
bemerybmw
commented
Apr 8, 2026
| /// TransactionLogLocalView::DereferenceSlotCallback created within TransactionLogSet::RollbackProxyTransactions and | ||
| /// RollbackSkeletonTracingTransactions. In these cases, the transaction will be recorded within | ||
| /// TransactionLog::RollbackIncrementTransactions resp. RollbackSubscribeTransactions before calling the callback. | ||
| void DereferenceEventWithoutTransactionLogging(const SlotIndexType event_slot_index) noexcept; |
Contributor
Author
There was a problem hiding this comment.
Remove this. This is only relevant to a consumer.
bemerybmw
commented
Apr 8, 2026
| /// which can negatively affect performance. Therefore, the data in EventDataControl is created / opened once during | ||
| /// Skeleton / Proxy creation, and then is accessed during runtime via EventDataControlLocal. | ||
| template <template <class> class AtomicIndirectorType = memory::shared::AtomicIndirectorReal> | ||
| class SkeletonEventDataControlLocalView final |
Contributor
Author
There was a problem hiding this comment.
Rename to ProviderEventDataControlLocalView
SampleAllocateePtr changed because the mock binding variant has been changed to also carry a custom deleter. Reflect this change by reusing the infrastructure of SamplePtr, whose mock variant uses the same pattern. Original author: darkwisebear
This commit splits up ServiceDataControl, EventDataControl and EventControl into a data class (ServiceDataControl, EventDataControl and EventControl) which contains the data that is placed in shared memory and behaviour "view" classes (ServiceDataControlLocalView, EventDataControlLocalView and EventControlLocalView) which contain behaviour for interacting with the data via process local raw pointers which avoid performance overhead of interacting with data directly in shared memory (due to performance overhead of dealing with OffsetPtrs).
Since we always create a QM EventDataControl, we pass it into the composite by reference instead of by pointer to avoid doing nullptr checks within the composite (and for clearer semantics).
We previously introduced these classes to avoid additional bounds checking done when indexing into a DynamicArray in shared memory. Since we now are using the local views into shared memory, we no longer need this optimisation and can revert to simply accessing slots using their indices.
Since the static_cast can only return a nullptr if the usable base address is a nullptr, we assert that the base address is not a nullptr as this is better reflecting the intention of the assertion.
ReferenceSpecificEvent has two assertions and corresponding tests for these assertions which are very similar. Additional documentation is added to the assertions and the tests are refactored to make the distinction clearer.
These Impl classes were previously added to avoid having to change calling code to explicitly use the default template argument (i.e. EventSubscriptionControl<>). Using the explicit template argument makes the code more explicit and also removes the additional code complexity of having the alias and Impl class.
We remove noexcept from all signatures to allow us to use precondition violation tests instead of death tests. Precondition tests are orders of magnitude faster than death tests due to not having to fork a process and also don't have issues associated with RAII objects being destroyed twice (i.e. in forked process and main process).
We now return a struct instead of a pair. This makes it easier to add additional return types in future (we will soon return a reference to a TransactionLogSet as well) and also makes it clearer on caller side as they can access members using the names defined in the struct rather than .first / .second. We also return a reference to EventDataStorage instead of a pointer, since it should always be created.
Updated documentation of functions and general clean up of functions.
We use ScopeExit so that we can rely only the more complicated testing of move construction / move assignment from the ScopeExit class.
This change will be needed in an upcoming PR in which TransactionLogSet::Register() creates a TransactionLogRegistrationGuard. Both of thes files will need TransactionLogIndex so to avoid cyclic dependencies we extract TransactionLogIndex into a separate file.
Previously, we were returning an index and then manually creating a guard from the index. We now return the guard directly. This is important because in an upcoming PR, the guard will be responsible for unregistering the TransactionLog with the TransactionLogSet and also destroying the cached TransactionLogLocalView in the Proxy/SkeletonEventDataControlView. Therefore, we want to make the guard responsible for all cleanup.
This commit implements multiple related changes. To fully split these changes into atomic commits would require a huge amount of work in temporary refactoring in production code and updating a large number of unit tests in each commit. Much of these changes would be temporary and simply overwritten in the next commit. Therefore, we combine the changes into a single commit. * Moves TransactionLogSet from EventDataControl to EventControl TransactionLogSet is currently stored in EventDataControl. The ownership semantics here don't make sense: the EventDataControl is responsible for managing slot reference counting and part of this is to record transactions in the TransactionLogSet. However, we also need to record subscription transactions which occurs outside of EventDataControl. Therefore, we move TransactionLogSet into EventControl to make it clearer that it's not owned by EventDataControl and to avoid having to have getters in ProxyEventDataControlLocalView / SkeletonEventDataControlLocalView for the TransactionLogSet which makes no sense semantically. * Cache TransactionLogViews in ProxyEventDataControlLocalView ProxyEventDataControlLocalView is responsible for doing the reference counting in EventDataControl in shared memory. These operations must be recorded in a TransactionLog so that these operations can be undone when restarting in case the process containing a Proxy crashes. This TransactionLogLocalView is now cached inside the ProxyEventDataControlLocalView on ProxyEvent subscription (and removed on unsubscription) to avoid having to look up the view in the TransactionLogSet every time it's required. * TransactionLogRegistrationGuard now also injects the TransactionLogLocalView in ProxyEventDataControlLocalView on construction and clears it on destruction The TransactionLogRegistrationGuard manages the lifetime of a registered TransactionLog. As long as it's alive (it's alive as long as a ProxyEvent is subscribed), we want the cached TransactionLogLocalView to be stored in ProxyEventDataControlLocalView. Therefore, the TransactionLogRegistrationGuard is now also responsible for injecting / clearing this TransactionLogLocalView. * Since ProxyEventDataControlLocalView now contains a cached TransactionLogLocalView, we no longer need to pass the TransactionLogIndex around to lookup TransactionLogs in TransactionLogSet Getters for TransactionLogIndex are removed from ProxyEventCommon / SubscriptionStateMachine. Classes such as SlotCollector, SlotDecrementer etc. no longer receive a TransactionLogIndex as argument.
Dereferencing is something inherent to a consumer. Therefore, it should only be called by a ProxyEventDataControlLocalView. When a Skeleton creates or opens a shared memory region, it will only create a ProxyEventDataControlLocalView if tracing is enabled in the current process. However, when opening a shared memory region from a crashed process, it's possible that tracing was enabled in that process but currently disabled. Therefore, we may not have created a ProxyEventDataControlLocalView even though we need one to rollback old transactions. Therefore, the Skeleton creates a new ProxyEventDataControlLocalView just to perform the rollback.
The two local views currently are split based on behaviour. ProxyEventDataControlLocalView is responsible for all "consumer" behaviour e.g. referencing / dereferencing a slot and SkeletonEventDataControlLocalView is responsible for all "provider" behaviour e.g. allocating slots. Due to IPC tracing, we have a unique scenario in which a SkeletonEvent acts like a consumer (i.e. it creates a SamplePtr which it uses to keep a slot alive while the tracing daemon is using it). In this case, it needs to use the "consumer" behaviour. Since we can have consumer behaviour also on skeleton side, it doesn't make sense to call the class ProxyEventDataControlLocalView. Therefore we rename them to have Consumer / Provider prefixes.
bemerybmw
force-pushed
the
brem_move_transaction_log_set
branch
from
e8a8109 to
0a3403b
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.
No description provided.