[SERVER-85325] Classify `OpCtx` decorations based on their performance cost Created: 17/Jan/24  Updated: 27/Jan/24  Resolved: 27/Jan/24

Status: Closed
Project: Core Server
Component/s: Internal Code
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Major - P3
Reporter: Amirsaman Memaripour Assignee: Patrick Freed
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: PNG File decoration-dtor.png     Text File decorations-by-binary.txt     Text File dtor-timings.txt     Zip Archive invisible-sharding-ycsb-flamegraphs.zip    
Issue Links:
Related
related to SERVER-85411 Evaluate the benefits of making `OpCt... Backlog
related to SERVER-81848 Make decoration construction eager again Closed
related to SERVER-76788 Elide useless constructor calls for t... Closed
is related to SERVER-85829 Remove OperationTimeTrackerHolder's u... In Code Review
is related to SERVER-85828 Optimize CurOpStack construction and ... In Progress
is related to SERVER-85830 Ensure the timers owner by OperationC... Closed
is related to SERVER-85831 Combine replication OperationContext ... Needs Scheduling
is related to SERVER-85832 Combine OpObserver decorations on Ope... Needs Scheduling
is related to SERVER-85833 Combine metadata OperationContext dec... Needs Scheduling
is related to SERVER-85834 Combine query OperationContext decora... Needs Scheduling
Backwards Compatibility: Fully Compatible
Sprint: Service Arch 2024-01-22, Service Arch 2024-02-05
Participants:

 Description   

The server makes a new OperationContext for each user operation. Instances of OperationContext are decorated, and making and destroying these decorations is expensive. This ticket should go over the list of all decorations on OperationContext and classify them into the following categories:

  • Trivially constructed and destructed, and already benefit from SERVER-76788.
  • Can be made cheaper by either making construction/destruction trivial, reusing decorations across multiple operations, simplifying the constructor/destructor, etc.
  • Other (i.e. decorations with complex construction/destruction logic that cannot be easily simplified).

Once we have this list, we can target the second and third groups by:

  • Filing a ticket to simplify decorations that belong to the second group.
  • File tickets for decorations that belong to the third group and assign it to the corresponding server team.


 Comments   
Comment by Patrick Freed [ 27/Jan/24 ]

Tickets have been filed, closing this out.

Comment by Patrick Freed [ 22/Jan/24 ]

I went and looked through the default constructors for all of OperationContexts decorations, and it doesn't look like any of them contain complex initialization logic. OperationTimeTrackerHolder 's constructor heap allocates an instance of a class that contains a mutex and a long, which could potentially be expensive if done on every operation, but other than that, the constructors are mostly just default and the member fields have simple or trivial constructors (e.g. integers, booleans, vectors, optionals).

The one notable decoration is CurOpStack, which by size is far larger than any other decoration on OperationContext. In fact, according to this comment, it constitutes over half of the entire decoration buffer. Its constructor also involves an extra small heap allocation. It would seem that the best avenue for improvement here would be to reduce the size of CurOpStack, perhaps by delegating non-required parts of it to separate allocations that we only do on a subset of operations or just by moving some of its state elsewhere. SERVER-81848 has a note about this and alex.li@mongodb.com mentioned that he'll be looking into it in a comment on that ticket, so I don't think anything separate needs to be filed at this time.

I would be curious to see a flamegraph or some benchmarking results that point to OperationContext decorations as being a performance bottleneck, since my initial look into it suggests that they shouldn't be overly expensive. The one exception could be CurOpStack, though even that is basically just a 3.5kb allocation, which doesn't seem egregious.

Here's the full list of decorations and some notes about them:

Trivial constructors/destructors

  • src/mongo/s/catalog_cache.cpp:106: bool
  • src/mongo/db/repl/always_allow_non_local_writes.cpp:40: bool
  • src/mongo/db/s/scoped_operation_completion_sharding_actions.cpp:69: bool

Simple / "empty" constructors

  • src/mongo/db/operation_cpu_timer.cpp:173: std::shared_ptr<OperationCPUTimers>
  • src/mongo/db/pipeline/javascript_execution.cpp:43: std::unique_ptr<JsExecution>
  • src/mongo/db/commands.cpp:634: boost::optional<BSONArray>
  • src/mongo/db/commands.cpp:886: std::shared_ptr<CommandInvocation>
  • src/mongo/db/session/session_catalog.cpp:67: boost::optional<SessionCatalog::ScopedCheckedOutSession>
  • src/mongo/db/serverless/shard_split_donor_op_observer.cpp:69: boost::optional<SplitCleanupDetails>
  • src/mongo/db/repl/tenant_migration_decoration.cpp:39: boost::optional<TenantMigrationInfo>
  • src/mongo/db/s/transaction_coordinator_worker_curop_repository_mongod.cpp:77: boost::optional<TransactionCoordinatorWorkerCurOpInfo>
  • src/mongo/db/query/plan_executor_impl.cpp:91: boost::optional<repl::OpTime>
  • src/mongo/db/query/plan_executor.cpp:52: boost::optional<SharedSemiFuture<void>>
  • src/mongo/db/auth/validated_tenancy_scope.cpp:48: boost::optional<ValidatedTenancyScope>
  • src/mongo/db/transaction_resources.cpp:52: std::unique_ptr<shard_role_details::TransactionResources>

Cheap but non-trivial/non-empty constructors

  • src/mongo/rpc/metadata/client_metadata.cpp:97: ClientMetadataState
      - bool, optional<ClientMetadata>
  • src/mongo/rpc/rewrite_state_change_errors.cpp:77: RewriteEnabled
      - one bool field
  • src/mongo/rpc/metadata/impersonated_user_metadata.cpp:57: synchronized_value<MaybeImpersonatedUserMetadata>
      - MaybeImpersonatedUserMetadata is a boost::optional
      - synchronized_value contains a mutex too
  • src/mongo/rpc/metadata/tracking_metadata.cpp:57: TrackingMetadata
      - a couple of boost::optionals and a bool
  • src/mongo/db/prepare_conflict_tracker.cpp:42: PrepareConflictTracker
      - two atomics and a tick
  • src/mongo/db/api_parameters.cpp:45: APIParameters
      - 3 boost::optionals
  • src/mongo/db/storage/storage_engine_change_context.cpp:83: StorageEngineChangeOperationContextDoneNotifier
      - condvar, mutex, int
  • src/mongo/db/repl/speculative_majority_read_info.cpp:53: SpeculativeMajorityReadInfo
      - bool, boost::optional
  • src/mongo/db/repl/repl_client_info.cpp:63: LastOpInfo
      - one bool
  • src/mongo/db/query/query_decorations.cpp:35: QueryKnobConfiguration
      - couple of bools and size_ts
  • src/mongo/db/query/find_common.cpp:72: AwaitDataState
      - one date_t and one bool
  • src/mongo/db/write_block_bypass.cpp:43: WriteBlockBypass
      - one bool
  • src/mongo/client/read_preference.cpp:88: ReadPreferenceSetting
      - read preference (enum + empty BSONObj)
      - tags (empty BSONObj)
      - Seconds
      - boost::optional<HedgingMode>
      - bool
  • src/mongo/db/catalog/document_validation.cpp:36: DocumentValidationSettings
      - one uint8_t
  • src/mongo/db/modules/enterprise/src/audit/audit_deduplicatoin.h:45: AuditDeduplication
      - one bool
  • src/mongo/db/storage/execution_context.cpp:38: StorageExecutionContext
      - two sets, a small_vector, and a vector
  • src/mongo/db/op_observer/batched_write_context.cpp:44: BatchedWriteContext
      - vector, set, two size_ts
      - does have multiple layers of indirection though, so a couple of function calls
  • src/mongo/db/op_observer/op_observer.cpp:39: OpObserver::Times
      - vector and an int
  • src/mongo/db/multi_key_path_tracker.cpp:54: MultikeyPathTracker
      - a vector and a bool
  • src/mongo/db/stats/resource_consumption_metrics.cpp:51: ResourceConsumption::MetricsCollector
      - medium amount of metrics, but again lots of small stuff like integers
  • src/mongo/db/s/operation_sharding_state.cpp:60: OperationShardingState
      - couple of maps, some bools
  • src/mongo/db/repl/read_concern_args.cpp:58: ReadConcernArgs
      - has a bunch of small fields

Potentially expensive constructors, could be made cheaper

  • src/mongo/db/operation_time_tracker.cpp:48: OperationTimeTrackerHolder
      - does heap allocation of mutex and a long
  • src/mongo/db/curop.cpp:210: CurOp::CurOpStack
      - tons of fields, although most are atomics integers or similar
      - also involves a heap allocation of a stats member
Generated at Thu Feb 08 06:57:27 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.