From 3b3ef49d402bc4a196fe0a76b778cd8e37822b1e Mon Sep 17 00:00:00 2001 From: Lukas Joswiak Date: Fri, 10 Jun 2022 16:53:19 -0700 Subject: [PATCH] Remove unnecessary transaction initialization `ReadYourWritesTransaction` has memory allocated before being passed to the main thread. This allows both threads to continue to access the transaction object. Currently, the transaction gets allocated and initialized on the foreign thread, and then re-initialized on the main thread. This causes a bunch of extra, unnecessary work for each `ReadYourWritesTransaction` where the temporary object gets destructed. The fix is to only allocate memory for the `ReadYourWritesTransaction` on the foreign thread, and then initialize it once on the main thread. --- fdbclient/ISingleThreadTransaction.cpp | 7 ++++++- fdbclient/ThreadSafeTransaction.cpp | 14 +++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/fdbclient/ISingleThreadTransaction.cpp b/fdbclient/ISingleThreadTransaction.cpp index e37213c149..df5fdef79e 100644 --- a/fdbclient/ISingleThreadTransaction.cpp +++ b/fdbclient/ISingleThreadTransaction.cpp @@ -26,9 +26,14 @@ ISingleThreadTransaction* ISingleThreadTransaction::allocateOnForeignThread(Type type) { if (type == Type::RYW) { - auto tr = new ReadYourWritesTransaction; + // We only want to allocate memory for the transaction, not initialize + // the object, so use operator new instead of new. The RYWTransaction + // will get initialized on the main thread. + auto tr = + (ReadYourWritesTransaction*)ReadYourWritesTransaction::operator new(sizeof(ReadYourWritesTransaction)); return tr; } else if (type == Type::SIMPLE_CONFIG) { + // Configuration transaction objects expect to be initialized. auto tr = new SimpleConfigTransaction; return tr; } else if (type == Type::PAXOS_CONFIG) { diff --git a/fdbclient/ThreadSafeTransaction.cpp b/fdbclient/ThreadSafeTransaction.cpp index 735637b4c3..32a6c527cd 100644 --- a/fdbclient/ThreadSafeTransaction.cpp +++ b/fdbclient/ThreadSafeTransaction.cpp @@ -190,12 +190,20 @@ ThreadSafeTransaction::ThreadSafeTransaction(DatabaseContext* cx, auto tr = this->tr = ISingleThreadTransaction::allocateOnForeignThread(type); // No deferred error -- if the construction of the RYW transaction fails, we have no where to put it onMainThreadVoid( - [tr, cx, tenant]() { + [tr, cx, type, tenant]() { cx->addref(); if (tenant.present()) { - tr->construct(Database(cx), tenant.get()); + if (type == ISingleThreadTransaction::Type::RYW) { + new (tr) ReadYourWritesTransaction(Database(cx), tenant.get()); + } else { + tr->construct(Database(cx), tenant.get()); + } } else { - tr->construct(Database(cx)); + if (type == ISingleThreadTransaction::Type::RYW) { + new (tr) ReadYourWritesTransaction(Database(cx)); + } else { + tr->construct(Database(cx)); + } } }, nullptr);