SERVER-20609 optimize OpDebug and CurOp construction

- replace assignment with NSDMI
- remove the giant ThreadSafeString from CurOp
- remove unnecessary reset() call to opDebug in assembleResponse

7 files changed, 86 insertions, 129 deletions

diff --git a/src/mongo/db/commands/count_cmd.cpp b/src/mongo/db/commands/count_cmd.cpp
index 5702dc1c2ca..3cd8985faca 100644
--- a/src/mongo/db/commands/count_cmd.cpp
+++ b/src/mongo/db/commands/count_cmd.cpp
@@ -31,6 +31,7 @@
 #include "mongo/platform/basic.h"
 
 
+#include "mongo/db/client.h"
 #include "mongo/db/commands.h"
 #include "mongo/db/curop.h"
 #include "mongo/db/db_raii.h"
@@ -148,8 +149,10 @@ public:
         unique_ptr<PlanExecutor> exec = std::move(statusWithPlanExecutor.getValue());
 
         // Store the plan summary string in CurOp.
-        if (NULL != CurOp::get(txn)) {
-            CurOp::get(txn)->debug().planSummary = Explain::getPlanSummary(exec.get());
+        {
+            auto curOp = CurOp::get(txn);
+            stdx::lock_guard<Client> lk(*txn->getClient());
+            curOp->setPlanSummary_inlock(Explain::getPlanSummary(exec.get()));
         }
 
         Status execPlanStatus = exec->executePlan();
diff --git a/src/mongo/db/curop.cpp b/src/mongo/db/curop.cpp
index b7f2105664e..148f3896b6d 100644
--- a/src/mongo/db/curop.cpp
+++ b/src/mongo/db/curop.cpp
@@ -236,19 +236,6 @@ CurOp::CurOp(OperationContext* opCtx, CurOpStack* stack) : _stack(stack) {
     } else {
         _stack->push_nolock(this);
     }
-    _start = 0;
-    _isCommand = false;
-    _dbprofile = 0;
-    _end = 0;
-    _maxTimeMicros = 0;
-    _maxTimeTracker.reset();
-    _message = "";
-    _progressMeter.finished();
-    _numYields = 0;
-    _expectedLatencyMs = 0;
-    _networkOp = opInvalid;
-    _logicalOp = opInvalid;
-    _command = NULL;
 }
 
 ProgressMeter& CurOp::setMessage_inlock(const char* msg,
@@ -326,8 +313,8 @@ void CurOp::reportState(BSONObjBuilder* builder) {
         _query.append(*builder, "query");
     }
 
-    if (!debug().planSummary.empty()) {
-        builder->append("planSummary", debug().planSummary.toString());
+    if (!_planSummary.empty()) {
+        builder->append("planSummary", _planSummary);
     }
 
     if (!_message.empty()) {
@@ -382,16 +369,6 @@ uint64_t CurOp::getRemainingMaxTimeMicros() const {
     return _maxTimeTracker.getRemainingMicros();
 }
 
-CurOp::MaxTimeTracker::MaxTimeTracker() {
-    reset();
-}
-
-void CurOp::MaxTimeTracker::reset() {
-    _enabled = false;
-    _targetEpochMicros = 0;
-    _approxTargetServerMillis = 0;
-}
-
 void CurOp::MaxTimeTracker::setTimeLimit(uint64_t startEpochMicros, uint64_t durationMicros) {
     dassert(durationMicros != 0);
 
@@ -453,43 +430,6 @@ uint64_t CurOp::MaxTimeTracker::getRemainingMicros() const {
     return _targetEpochMicros - now;
 }
 
-void OpDebug::reset() {
-    networkOp = opInvalid;
-    logicalOp = opInvalid;
-    iscommand = false;
-    query = BSONObj();
-    updateobj = BSONObj();
-
-    cursorid = -1;
-    ntoreturn = -1;
-    ntoskip = -1;
-    exhaust = false;
-
-    keysExamined = -1;
-    docsExamined = -1;
-    idhack = false;
-    hasSortStage = false;
-    nMatched = -1;
-    nModified = -1;
-    ninserted = -1;
-    ndeleted = -1;
-    nmoved = -1;
-    fastmod = false;
-    fastmodinsert = false;
-    upsert = false;
-    cursorExhausted = false;
-    keyUpdates = 0;  // unsigned, so -1 not possible
-    writeConflicts = 0;
-    planSummary = "";
-    execStats.reset();
-
-    exceptionInfo.reset();
-
-    executionTime = 0;
-    nreturned = -1;
-    responseLength = -1;
-}
-
 namespace {
 StringData getProtoString(int op) {
     if (op == dbQuery) {
@@ -535,8 +475,8 @@ string OpDebug::report(const CurOp& curop, const SingleThreadedLockStats& lockSt
         }
     }
 
-    if (!planSummary.empty()) {
-        s << " planSummary: " << planSummary.toString();
+    if (!curop.getPlanSummary().empty()) {
+        s << " planSummary: " << curop.getPlanSummary();
     }
 
     if (!updateobj.isEmpty()) {
diff --git a/src/mongo/db/curop.h b/src/mongo/db/curop.h
index bcdfb3de38f..55e48df56cf 100644
--- a/src/mongo/db/curop.h
+++ b/src/mongo/db/curop.h
@@ -129,11 +129,7 @@ private:
 /* lifespan is different than CurOp because of recursives with DBDirectClient */
 class OpDebug {
 public:
-    OpDebug() : planSummary(2048) {
-        reset();
-    }
-
-    void reset();
+    OpDebug() = default;
 
     std::string report(const CurOp& curop, const SingleThreadedLockStats& lockStats) const;
 
@@ -151,38 +147,42 @@ public:
 
     // basic options
     // _networkOp represents the network-level op code: OP_QUERY, OP_GET_MORE, OP_COMMAND, etc.
-    Operation networkOp;  // only set this through setNetworkOp_inlock() to keep synced
+    Operation networkOp{opInvalid};  // only set this through setNetworkOp_inlock() to keep synced
     // _logicalOp is the logical operation type, ie 'dbQuery' regardless of whether this is an
     // OP_QUERY find, a find command using OP_QUERY, or a find command using OP_COMMAND.
     // Similarly, the return value will be dbGetMore for both OP_GET_MORE and getMore command.
-    Operation logicalOp;  // only set this through setNetworkOp_inlock() to keep synced
-    bool iscommand;
-    BSONObj query;
-    BSONObj updateobj;
+    Operation logicalOp{opInvalid};  // only set this through setNetworkOp_inlock() to keep synced
+    bool iscommand{false};
+    BSONObj query{};
+    BSONObj updateobj{};
 
     // detailed options
-    long long cursorid;
-    long long ntoreturn;
-    long long ntoskip;
-    bool exhaust;
+    long long cursorid{-1};
+    long long ntoreturn{-1};
+    long long ntoskip{-1};
+    bool exhaust{false};
 
     // debugging/profile info
-    long long keysExamined;
-    long long docsExamined;
-    bool idhack;  // indicates short circuited code path on an update to make the update faster
-    bool hasSortStage;    // true if the query plan involves an in-memory sort
-    long long nMatched;   // number of records that match the query
-    long long nModified;  // number of records written (no no-ops)
-    long long nmoved;     // updates resulted in a move (moves are expensive)
-    long long ninserted;
-    long long ndeleted;
-    bool fastmod;
-    bool fastmodinsert;    // upsert of an $operation. builds a default object
-    bool upsert;           // true if the update actually did an insert
-    bool cursorExhausted;  // true if the cursor has been closed at end a find/getMore operation
-    int keyUpdates;
-    long long writeConflicts;
-    ThreadSafeString planSummary;  // a brief std::string describing the query solution
+    long long keysExamined{-1};
+    long long docsExamined{-1};
+
+    // indicates short circuited code path on an update to make the update faster
+    bool idhack{false};
+
+    bool hasSortStage{false};  // true if the query plan involves an in-memory sort
+
+    long long nMatched{-1};   // number of records that match the query
+    long long nModified{-1};  // number of records written (no no-ops)
+    long long nmoved{-1};     // updates resulted in a move (moves are expensive)
+    long long ninserted{-1};
+    long long ndeleted{-1};
+    bool fastmod{false};
+    bool fastmodinsert{false};  // upsert of an $operation. builds a default object
+    bool upsert{false};         // true if the update actually did an insert
+    bool cursorExhausted{
+        false};  // true if the cursor has been closed at end a find/getMore operation
+    int keyUpdates{0};
+    long long writeConflicts{0};
 
     // New Query Framework debugging/profiling info
     // TODO: should this really be an opaque BSONObj?  Not sure.
@@ -192,9 +192,9 @@ public:
     ExceptionInfo exceptionInfo;
 
     // response info
-    int executionTime;
-    long long nreturned;
-    int responseLength;
+    int executionTime{0};
+    long long nreturned{-1};
+    int responseLength{-1};
 
 private:
     /**
@@ -473,6 +473,18 @@ public:
      */
     void setNS_inlock(StringData ns);
 
+    StringData getPlanSummary() const {
+        return _planSummary;
+    }
+
+    void setPlanSummary_inlock(StringData summary) {
+        _planSummary = summary.toString();
+    }
+
+    void setPlanSummary_inlock(std::string summary) {
+        _planSummary = std::move(summary);
+    }
+
 private:
     class CurOpStack;
 
@@ -481,34 +493,36 @@ private:
     CurOp(OperationContext*, CurOpStack*);
 
     CurOpStack* _stack;
-    CurOp* _parent = nullptr;
-    Command* _command;
-    long long _start;
-    long long _end;
+    CurOp* _parent{nullptr};
+    Command* _command{nullptr};
+    long long _start{0};
+    long long _end{0};
 
     // _networkOp represents the network-level op code: OP_QUERY, OP_GET_MORE, OP_COMMAND, etc.
-    Operation _networkOp;  // only set this through setNetworkOp_inlock() to keep synced
+    Operation _networkOp{opInvalid};  // only set this through setNetworkOp_inlock() to keep synced
     // _logicalOp is the logical operation type, ie 'dbQuery' regardless of whether this is an
     // OP_QUERY find, a find command using OP_QUERY, or a find command using OP_COMMAND.
     // Similarly, the return value will be dbGetMore for both OP_GET_MORE and getMore command.
-    Operation _logicalOp;  // only set this through setNetworkOp_inlock() to keep synced
+    Operation _logicalOp{opInvalid};  // only set this through setNetworkOp_inlock() to keep synced
 
-    bool _isCommand;
-    int _dbprofile;  // 0=off, 1=slow, 2=all
+    bool _isCommand{false};
+    int _dbprofile{0};  // 0=off, 1=slow, 2=all
     std::string _ns;
     CachedBSONObj<512> _query;  // CachedBSONObj is thread safe
     OpDebug _debug;
     std::string _message;
     ProgressMeter _progressMeter;
-    int _numYields;
+    int _numYields{0};
 
     // this is how much "extra" time a query might take
     // a writebacklisten for example will block for 30s
     // so this should be 30000 in that case
-    long long _expectedLatencyMs;
+    long long _expectedLatencyMs{0};
 
     // Time limit for this operation.  0 if the operation has no time limit.
-    uint64_t _maxTimeMicros;
+    uint64_t _maxTimeMicros{0u};
+
+    std::string _planSummary;
 
     /** Nested class that implements tracking of a time limit for a CurOp object. */
     class MaxTimeTracker {
@@ -516,10 +530,7 @@ private:
 
     public:
         /** Newly-constructed MaxTimeTracker objects have the time limit disabled. */
-        MaxTimeTracker();
-
-        /** Disables the time tracker. */
-        void reset();
+        MaxTimeTracker() = default;
 
         /** Returns whether or not time tracking is enabled. */
         bool isEnabled() const {
@@ -551,15 +562,15 @@ private:
 
     private:
         // Whether or not time tracking is enabled for this operation.
-        bool _enabled;
+        bool _enabled{false};
 
         // Point in time at which the time limit is hit.  Units of microseconds since the
         // epoch.
-        uint64_t _targetEpochMicros;
+        uint64_t _targetEpochMicros{0};
 
         // Approximate point in time at which the time limit is hit.   Units of milliseconds
         // since the server process was started.
-        int64_t _approxTargetServerMillis;
+        int64_t _approxTargetServerMillis{0};
     } _maxTimeTracker;
 };
 }
diff --git a/src/mongo/db/instance.cpp b/src/mongo/db/instance.cpp
index e752ae92fa6..bc6481410f1 100644
--- a/src/mongo/db/instance.cpp
+++ b/src/mongo/db/instance.cpp
@@ -615,7 +615,6 @@ void assembleResponse(OperationContext* txn,
     }
 
     recordCurOpMetrics(txn);
-    debug.reset();
 }
 
 void receivedKillCursors(OperationContext* txn, Message& m) {
diff --git a/src/mongo/db/query/find.cpp b/src/mongo/db/query/find.cpp
index 2e863dc1b5e..62da32cacf2 100644
--- a/src/mongo/db/query/find.cpp
+++ b/src/mongo/db/query/find.cpp
@@ -168,7 +168,8 @@ void endQueryOp(OperationContext* txn,
     if (dbProfilingLevel > 0 || curop->elapsedMillis() > serverGlobalParams.slowMS ||
         logger::globalLogDomain()->shouldLog(queryLogComponent, logLevelOne)) {
         // Generate plan summary string.
-        curop->debug().planSummary = Explain::getPlanSummary(&exec);
+        stdx::lock_guard<Client>(*txn->getClient());
+        curop->setPlanSummary_inlock(Explain::getPlanSummary(&exec));
     }
 
     // Set debug information for consumption by the profiler only.
@@ -180,9 +181,9 @@ void endQueryOp(OperationContext* txn,
         curop->debug().execStats.set(statsBob.obj());
 
         // Replace exec stats with plan summary if stats cannot fit into CachedBSONObj.
-        if (curop->debug().execStats.tooBig() && !curop->debug().planSummary.empty()) {
+        if (curop->debug().execStats.tooBig() && !curop->getPlanSummary().empty()) {
             BSONObjBuilder bob;
-            bob.append("summary", curop->debug().planSummary.toString());
+            bob.append("summary", curop->getPlanSummary());
             curop->debug().execStats.set(bob.done());
         }
     }
@@ -579,7 +580,10 @@ std::string runQuery(OperationContext* txn,
     // uint64_t numMisplacedDocs = 0;
 
     // Get summary info about which plan the executor is using.
-    curop.debug().planSummary = Explain::getPlanSummary(exec.get());
+    {
+        stdx::lock_guard<Client> lk(*txn->getClient());
+        curop.setPlanSummary_inlock(Explain::getPlanSummary(exec.get()));
+    }
 
     while (PlanExecutor::ADVANCED == (state = exec->getNext(&obj, NULL))) {
         // Add result to output buffer.
diff --git a/src/mongo/util/progress_meter.cpp b/src/mongo/util/progress_meter.cpp
index 50575aff98a..2249c836750 100644
--- a/src/mongo/util/progress_meter.cpp
+++ b/src/mongo/util/progress_meter.cpp
@@ -49,7 +49,7 @@ void ProgressMeter::reset(unsigned long long total, int secondsBetween, int chec
     _hits = 0;
     _lastTime = (int)time(0);
 
-    _active = 1;
+    _active = true;
 }
 
 
diff --git a/src/mongo/util/progress_meter.h b/src/mongo/util/progress_meter.h
index 50042bfadcc..3dca5350692 100644
--- a/src/mongo/util/progress_meter.h
+++ b/src/mongo/util/progress_meter.h
@@ -49,7 +49,7 @@ public:
         reset(total, secondsBetween, checkInterval);
     }
 
-    ProgressMeter() : _active(0), _showTotal(true), _units("") {
+    ProgressMeter() {
         _name = "Progress";
     }
 
@@ -57,7 +57,7 @@ public:
     void reset(unsigned long long total, int secondsBetween = 3, int checkInterval = 100);
 
     void finished() {
-        _active = 0;
+        _active = false;
     }
     bool isActive() const {
         return _active;
@@ -110,12 +110,12 @@ public:
     }
 
 private:
-    bool _active;
+    bool _active{false};
 
     unsigned long long _total;
-    bool _showTotal;
-    int _secondsBetween;
-    int _checkInterval;
+    bool _showTotal{true};
+    int _secondsBetween{3};
+    int _checkInterval{100};
 
     unsigned long long _done;
     unsigned long long _hits;