[SERVER-76788] Elide useless constructor calls for trivially-constructible Decorations Created: 03/May/23  Updated: 22/Jan/24  Resolved: 06/Jun/23

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: 7.1.0-rc0

Type: Improvement Priority: Major - P3
Reporter: Andy Schwerin Assignee: Billy Donahue
Resolution: Fixed Votes: 0
Labels: perf-servicearch
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: Text File decorations_all.txt     PNG File screenshot-1.png    
Issue Links:
Backports
Depends
Related
related to SERVER-77825 optimize Decorations that are initial... Closed
is related to SERVER-85325 Classify `OpCtx` decorations based on... Closed
is related to SERVER-69383 Improve performance of Decorations Closed
Assigned Teams:
Service Arch
Backwards Compatibility: Fully Compatible
Backport Requested:
v7.0
Sprint: Service Arch 2023-05-15, Service Arch 2023-05-29, Service Arch 2023-06-12
Participants:

 Description   

Decoration construction, especially for OperationContext takes a significant amount of time, as there is a very high number of decorations on that type. The cost comes from virtual function dispatch to invoke the constructors. Many of the decorations are trivially constructible and so don't really need any action taken at construction. The task of this ticket is to figure out how to avoid invoking constructor functions that don't need to do any work in order to speed up the construction and destruction of Decorable objects. Presumably this would be similar to SERVER-69383.

If we cannot do this, we should instead investigate combining related decorations into structures to reduce the number of constructor/destructor actions.



 Comments   
Comment by Ger Hartnett [ 11/Sep/23 ]

kyle.suarez@mongodb.com said:

SERVER-77825 builds on the work of this ticket ... I am of the opinion that we should favor backporting this ticket (and potentially that ticket)

david.daly@mongodb.com agreed.

Kyle did you mean backporting this to 7.0? If so, do we need a BACKPORT ticket?

Comment by David Daly [ 06/Jun/23 ]

SGTM kyle.suarez@mongodb.com 

Comment by Kyle Suarez [ 31/May/23 ]

billy.donahue@mongodb.com / blake.oler@mongodb.com, is there more work to be done on this ticket or should it be closed out?

Comment by Githook User [ 19/May/23 ]

Author:

{'name': 'Billy Donahue', 'email': 'billy.donahue@mongodb.com', 'username': 'BillyDonahue'}

Message: SERVER-76788 follow-on: zero-initialize copyable decoration containers
Branch: master
https://github.com/mongodb/mongo/commit/85c0e6005c761ebb6886f5957b367a2e70eb3524

Comment by Billy Donahue [ 18/May/23 ]

initial commit left out the copyable decoration containers.

Comment by Billy Donahue [ 18/May/23 ]

decorations_all.txt
Here's a list of ALL the decorations in mongod. 12 of the 257 are trivially constructible.

They are:

trivial = true    , D = mongo::CollectionShardingRuntime    , T = mongo::MigrationSourceManager*
trivial = true    , D = mongo::OperationContext             , T = bool
trivial = true    , D = mongo::OperationContext             , T = bool
trivial = true    , D = mongo::OperationContext             , T = bool
trivial = true    , D = mongo::OperationContext             , T = bool
trivial = true    , D = mongo::OperationContext             , T = bool
trivial = true    , D = mongo::OperationContext             , T = bool
trivial = true    , D = mongo::ServiceContext               , T = bool
trivial = true    , D = mongo::ServiceContext               , T = bool
trivial = true    , D = mongo::ServiceContext               , T = mongo::VectorClock*
trivial = true    , D = mongo::ServiceContext               , T = mongo::VectorClockMutable*
trivial = true    , D = mongo::ServiceContext               , T = mongo::analyze_shard_key::QueryAnalysisClient

Comment by Billy Donahue [ 17/May/23 ]

I'm kind of skeptical of my own results because this should only affect trivially constructible decorations, and I don't see that many std::trivially_constructible decorations on OperationContext the only per-command decorable.

Doing a little survey now to see just how many decorations this affected and didn't affect.

Here are all the decorations on OperationContext, from an instrumented mongod that prints a log line in declareDecoration.

boost::optional<bool>: trivial=0
boost::optional<mongo::BSONArray>: trivial=0
boost::optional<mongo::SessionCatalog::ScopedCheckedOutSession>: trivial=0
boost::optional<mongo::ShardId>: trivial=0
boost::optional<mongo::SharedSemiFuture<void> >: trivial=0
boost::optional<mongo::TenantId>: trivial=0
boost::optional<mongo::auth::ValidatedTenancyScope>: trivial=0
boost::optional<mongo::repl::DocumentKey>: trivial=0
boost::optional<mongo::repl::TenantMigrationInfo>: trivial=0
boost::optional<mongo::{anonymous}::SplitCleanupDetails>: trivial=0
boost::optional<mongo::{anonymous}::TransactionCoordinatorWorkerCurOpInfo>: trivial=0
mongo::APIParameters: trivial=0
mongo::AwaitDataState: trivial=0
mongo::BSONObj: trivial=0
mongo::BSONObj: trivial=0
mongo::BSONObj: trivial=0
mongo::BSONObj: trivial=0
mongo::BSONObj: trivial=0
mongo::BSONObj: trivial=0
mongo::BatchedWriteContext: trivial=0
mongo::CurOp::CurOpStack: trivial=0
mongo::DocumentValidationSettings: trivial=0
mongo::MultikeyPathTracker: trivial=0
mongo::OpObserver::Times: trivial=0
mongo::OperationKeyManager; D = mongo::ServiceContext: trivial=0
mongo::OperationShardingState: trivial=0
mongo::PrepareConflictTracker: trivial=0
mongo::ReadPreferenceSetting: trivial=0
mongo::ResourceConsumption::MetricsCollector: trivial=0
mongo::ServerlessOperationLockRegistry; D = mongo::ServiceContext: trivial=0
mongo::StorageEngineChangeOperationContextDoneNotifier: trivial=0
mongo::StorageExecutionContext: trivial=0
mongo::WriteBlockBypass: trivial=0
mongo::audit::{anonymous}::AuditDeduplication: trivial=0
mongo::repl::OpTime: trivial=0
mongo::repl::ReadConcernArgs: trivial=0
mongo::repl::SpeculativeMajorityReadInfo: trivial=0
mongo::repl::{anonymous}::LastOpInfo: trivial=0
mongo::rpc::TrackingMetadata: trivial=0
mongo::rpc::{anonymous}::RewriteEnabled: trivial=0
mongo::synchronized_value<boost::optional<mongo::rpc::ImpersonatedUserMetadata> >: trivial=0
mongo::{anonymous}::ClientMetadataState: trivial=0
mongo::{anonymous}::OperationTimeTrackerHolder: trivial=0
std::__cxx11::basic_string<char>: trivial=0
std::shared_ptr<mongo::CommandInvocation>: trivial=0
std::shared_ptr<mongo::OperationCPUTimers>: trivial=0
std::unique_ptr<mongo::JsExecution>: trivial=0
std::unique_ptr<mongo::LDAPCumulativeOperationStats>; D = mongo::ServiceContext: trivial=0
std::unique_ptr<mongo::shard_role_details::TransactionResources>: trivial=0
 
bool: trivial=1
bool: trivial=1
bool: trivial=1
bool: trivial=1
bool: trivial=1
bool: trivial=1

The ONLY trivial decorations are the 6 out of 55 (11%) that are bool.
So only a few of the decorations became trivially constructed after this change.

This needs more investigation.

Comment by Githook User [ 17/May/23 ]

Author:

{'name': 'Billy Donahue', 'email': 'billy.donahue@mongodb.com', 'username': 'BillyDonahue'}

Message: SERVER-76788 elide trivial Decoration constructors
Branch: master
https://github.com/mongodb/mongo/commit/fa994d0377ddc7a10b760a5bde75239d76b7f1e7

Comment by Billy Donahue [ 16/May/23 ]

Whoa.

sys-perf

Comment by Billy Donahue [ 12/May/23 ]

irina.yatsenko@mongodb.com interesting signal boost for this ticket, thanks!

Comment by Irina Yatsenko (Inactive) [ 11/May/23 ]

Constructing class Snapshot : public Decorable<Snapshot> is responsible for some (1-2%) of the regression in https://jira.mongodb.org/browse/PERF-3818 (updateMany with no filter on a collection with 50K docs)

Comment by Andy Schwerin [ 03/May/23 ]

We should also microbenchmark the construction of OperationContext and Client as decorated inside mongod in particular.

Comment by Billy Donahue [ 03/May/23 ]

SERVER-69383 did this for trivial destructors.
We should have done constructors too.

Generated at Thu Feb 08 06:33:36 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.