diff --git a/hphp/runtime/ext/vsdebug/debugger.cpp b/hphp/runtime/ext/vsdebug/debugger.cpp index 7c286f7f00abec..fe643a34fb5fb8 100644 --- a/hphp/runtime/ext/vsdebug/debugger.cpp +++ b/hphp/runtime/ext/vsdebug/debugger.cpp @@ -329,6 +329,9 @@ void Debugger::cleanupRequestInfo(ThreadInfo* ti, RequestInfo* ri) { delete ri->m_breakpointInfo; } + ri->m_scopeIds.clear(); + ri->m_objectIds.clear(); + assertx(ri->m_serverObjects.size() == 0); delete ri; } diff --git a/hphp/runtime/ext/vsdebug/debugger.h b/hphp/runtime/ext/vsdebug/debugger.h index f6492b8101baa9..d31c56304f1433 100644 --- a/hphp/runtime/ext/vsdebug/debugger.h +++ b/hphp/runtime/ext/vsdebug/debugger.h @@ -97,6 +97,10 @@ struct RequestInfo { const char* m_stepReason {nullptr}; CommandQueue m_commandQueue; RequestBreakpointInfo* m_breakpointInfo {nullptr}; + + // Object IDs sent to the debugger client. + std::unordered_map m_scopeIds; + std::unordered_map m_objectIds; std::unordered_map m_serverObjects; struct { @@ -177,6 +181,9 @@ struct DebuggerOptions { // Tell the user if breakpoint calibration moves their bp. bool notifyOnBpCalibration; + + // Don't try to unique variable references by address. + bool disableUniqueVarRef; }; struct ClientInfo { @@ -469,11 +476,13 @@ struct Debugger final { VSDebugLogger::LogLevelInfo, "Client options set:\n" "showDummyOnAsyncPause: %s\n" - "warnOnInterceptedFunctions: %s\n", - "notifyOnBpCalibration: %s\n", + "warnOnInterceptedFunctions: %s\n" + "notifyOnBpCalibration: %s\n" + "disableUniqueVarRef: %s\n", options.showDummyOnAsyncPause ? "YES" : "NO", options.warnOnInterceptedFunctions ? "YES" : "NO", - options.notifyOnBpCalibration ? "YES" : "NO" + options.notifyOnBpCalibration ? "YES" : "NO", + options.disableUniqueVarRef ? "YES" : "NO" ); } diff --git a/hphp/runtime/ext/vsdebug/launch_attach_command.cpp b/hphp/runtime/ext/vsdebug/launch_attach_command.cpp index 53cc286678c337..ad8af24591fb97 100644 --- a/hphp/runtime/ext/vsdebug/launch_attach_command.cpp +++ b/hphp/runtime/ext/vsdebug/launch_attach_command.cpp @@ -56,6 +56,9 @@ bool LaunchAttachCommand::executeImpl(DebuggerSession* /*session*/, bool notifyOnBpCalibration = tryGetBool(args, "notifyOnBpCalibration", false); + bool disableUniqueVarRef = + tryGetBool(args, "disableUniqueVarRef", false); + const auto& logFilePath = tryGetString(args, "logFilePath", emptyString); @@ -94,6 +97,7 @@ bool LaunchAttachCommand::executeImpl(DebuggerSession* /*session*/, options.showDummyOnAsyncPause = showDummyOnAsyncPause; options.warnOnInterceptedFunctions = warnOnInterceptedFunctions; options.notifyOnBpCalibration = notifyOnBpCalibration; + options.disableUniqueVarRef = disableUniqueVarRef; m_debugger->setDebuggerOptions(options); // Send the InitializedEvent to indicate to the front-end that we are up diff --git a/hphp/runtime/ext/vsdebug/session.cpp b/hphp/runtime/ext/vsdebug/session.cpp index 44f269f26fcb6a..f6ecde64ce2930 100644 --- a/hphp/runtime/ext/vsdebug/session.cpp +++ b/hphp/runtime/ext/vsdebug/session.cpp @@ -304,9 +304,20 @@ unsigned int DebuggerSession::generateScopeId( int depth, ScopeType scopeType ) { - const unsigned int objectId = ++s_nextObjectId; - ScopeObject* scope = new ScopeObject(objectId, requestId, depth, scopeType); + unsigned int objectId; + RequestInfo* ri = m_debugger->getRequestInfo(); + auto& existingScopes = ri->m_scopeIds; + auto it = existingScopes.find((int)scopeType); + if (it != existingScopes.end()) { + // This scope type for this request already has an ID assigned. + // Reuse it. + objectId = it->second; + } else { + objectId = ++s_nextObjectId; + existingScopes.emplace((int)scopeType, objectId); + } + ScopeObject* scope = new ScopeObject(objectId, requestId, depth, scopeType); assertx(requestId == m_debugger->getCurrentThreadId()); registerRequestObject(objectId, scope); return objectId; @@ -316,9 +327,8 @@ unsigned int DebuggerSession::generateVariableId( request_id_t requestId, Variant& variable ) { - const unsigned int objectId = ++s_nextObjectId; + const unsigned int objectId = generateOrReuseVariableId(variable); VariableObject* varObj = new VariableObject(objectId, requestId, variable); - assertx(requestId == m_debugger->getCurrentThreadId()); registerRequestObject(objectId, varObj); return objectId; @@ -345,6 +355,47 @@ unsigned int DebuggerSession::generateVariableSubScope( return objectId; } +unsigned int DebuggerSession::generateOrReuseVariableId( + const Variant& variable +) { + // Generate a new object ID if we haven't seen this variant before. Ensure + // IDs are stable for variants that contain objects or arrays, based on the + // address of the object to which they point. + RequestInfo* ri = m_debugger->getRequestInfo(); + const auto options = m_debugger->getDebuggerOptions(); + + void* key = nullptr; + auto& existingVariables = ri->m_objectIds; + if (!options.disableUniqueVarRef) { + if (variable.isArray()) { + key = (void*)variable.getArrayDataOrNull(); + } else if (variable.isObject()) { + key = (void*)variable.getObjectDataOrNull(); + } + + if (key != nullptr) { + const auto it = existingVariables.find(key); + if (it != existingVariables.end()) { + const unsigned int objectId = it->second; + return objectId; + } + } + } + + // Allocate a new ID. + const unsigned int objectId = ++s_nextObjectId; + + if (key != nullptr) { + // Remember the object ID for complex types (Objects, Arrays). + // Since simple types have no children, and cannot be expanded + // in clients' Variables/Scopes windows, it is not important + // that they receive the same object ID across requests. + existingVariables.emplace(key, objectId); + } + + return objectId; +} + ScopeObject* DebuggerSession::getScopeObject(unsigned int objectId) { auto object = getServerObject(objectId); if (object != nullptr) { @@ -365,7 +416,16 @@ void DebuggerSession::registerRequestObject( RequestInfo* ri = m_debugger->getRequestInfo(); // Add this object to the per-request list of objects. - std::unordered_map& objs = ri->m_serverObjects; + auto& objs = ri->m_serverObjects; + auto it = objs.find(objectId); + if (it != objs.end()) { + // Replacing server object by ID. Free the old object. + ServerObject* object = it->second; + objs.erase(it); + onServerObjectDestroyed(objectId); + delete object; + } + objs.emplace(objectId, obj); // Add this object to the per-session global list of objects. diff --git a/hphp/runtime/ext/vsdebug/session.h b/hphp/runtime/ext/vsdebug/session.h index 250f39e6e6c350..0b379cc347a54e 100644 --- a/hphp/runtime/ext/vsdebug/session.h +++ b/hphp/runtime/ext/vsdebug/session.h @@ -95,6 +95,10 @@ struct DebuggerSession final { private: + unsigned int generateOrReuseVariableId( + const Variant& variable + ); + void registerRequestObject( unsigned int objectId, ServerObject* obj @@ -120,8 +124,7 @@ struct DebuggerSession final { // When a request is paused, the backend must maintain state about scopes, // frames and variable IDs sent to the front end. The IDs are globally - // unique, and the objects to which they refer are valid per-request and - // only until that request resumes. + // unique, and the objects to which they refer are valid per-request.. static unsigned int s_nextObjectId; std::unordered_map m_serverObjects; diff --git a/hphp/test/slow/ext_vsdebug/context.php b/hphp/test/slow/ext_vsdebug/context.php index 263883ac4720bd..7b1a101c0b59f6 100644 --- a/hphp/test/slow/ext_vsdebug/context.php +++ b/hphp/test/slow/ext_vsdebug/context.php @@ -150,7 +150,7 @@ $seq = sendVsCommand(array( "command" => "variables", "type" => "request", - "arguments" => array("variablesReference" => 10))); + "arguments" => array("variablesReference" => 3))); $msg = json_decode(getNextVsDebugMessage(), true); checkObjEqualRecursively($msg, array( "type" => "response", @@ -202,7 +202,7 @@ $seq = sendVsCommand(array( "command" => "variables", "type" => "request", - "arguments" => array("variablesReference" => 17))); + "arguments" => array("variablesReference" => 14))); $msg = json_decode(getNextVsDebugMessage(), true); checkObjEqualRecursively($msg, array( @@ -217,7 +217,7 @@ "name" => "\$aObj", "value" => "A", "namedVariables" => 2, - "variablesReference" => 19, + "variablesReference" => 16, "presentationHint" => array( "visibility" => "public" ) @@ -241,7 +241,7 @@ // The private props should contain the base class's copy of $a, only. array( - "variablesReference" => 20, + "variablesReference" => 17, "name" => "Private props", "value" => "class A", "namedVariables" => 1, @@ -253,7 +253,7 @@ // Two constants should be visible on A, HELLOA and HELLOB, and one // on class B. array( - "variablesReference" => 21, + "variablesReference" => 18, "name" => "Class Constants", "value" => "class B", "namedVariables" => 3, @@ -263,7 +263,7 @@ ), array( - "variablesReference" => 22, + "variablesReference" => 19, "name" => "Static Props", "value" => "class B", "namedVariables" => 1, @@ -278,7 +278,7 @@ $seq = sendVsCommand(array( "command" => "variables", "type" => "request", - "arguments" => array("variablesReference" => 19))); + "arguments" => array("variablesReference" => 16))); $msg = json_decode(getNextVsDebugMessage(), true); checkObjEqualRecursively($msg, array( @@ -332,7 +332,7 @@ $seq = sendVsCommand(array( "command" => "variables", "type" => "request", - "arguments" => array("variablesReference" => 20))); + "arguments" => array("variablesReference" => 17))); $msg = json_decode(getNextVsDebugMessage(), true); checkObjEqualRecursively($msg, array( "type" => "response", @@ -360,7 +360,7 @@ $seq = sendVsCommand(array( "command" => "variables", "type" => "request", - "arguments" => array("variablesReference" => 22))); + "arguments" => array("variablesReference" => 19))); $msg = json_decode(getNextVsDebugMessage(), true); checkObjEqualRecursively($msg, array( "type" => "response", @@ -388,7 +388,7 @@ $seq = sendVsCommand(array( "command" => "variables", "type" => "request", - "arguments" => array("variablesReference" => 21))); + "arguments" => array("variablesReference" => 18))); $msg = json_decode(getNextVsDebugMessage(), true); checkObjEqualRecursively($msg, array( "type" => "response", @@ -432,7 +432,7 @@ $seq = sendVsCommand(array( "command" => "variables", "type" => "request", - "arguments" => array("variablesReference" => 15))); + "arguments" => array("variablesReference" => 12))); $msg = json_decode(getNextVsDebugMessage(), true); checkObjEqualRecursively($msg, array( "type" => "response", @@ -467,7 +467,7 @@ $seq = sendVsCommand(array( "command" => "variables", "type" => "request", - "arguments" =>array("variablesReference" => 15, "count" => 2))); + "arguments" =>array("variablesReference" => 12, "count" => 2))); $msg = json_decode(getNextVsDebugMessage(), true); checkObjEqualRecursively($msg, array( "type" => "response", @@ -497,7 +497,7 @@ $seq = sendVsCommand(array( "command" => "variables", "type" => "request", - "arguments" => array("variablesReference" => 16))); + "arguments" => array("variablesReference" => 13))); $msg = json_decode(getNextVsDebugMessage(), true); checkObjEqualRecursively($msg, array( "type" => "response", @@ -515,7 +515,7 @@ "type" => "array", "name" => "1", "value" => "array[2]", - "variablesReference" => 25 + "variablesReference" => 22 ) ] ])); @@ -528,7 +528,7 @@ $seq = sendVsCommand(array( "command" => "variables", "type" => "request", - "arguments" => array("variablesReference" => 25))); + "arguments" => array("variablesReference" => 22))); $msg = json_decode(getNextVsDebugMessage(), true); checkObjEqualRecursively($msg, array( "type" => "response", @@ -558,7 +558,7 @@ $seq = sendVsCommand(array( "command" => "variables", "type" => "request", - "arguments" => array("variablesReference" => 18))); + "arguments" => array("variablesReference" => 15))); $msg = json_decode(getNextVsDebugMessage(), true); checkObjEqualRecursively($msg, array( "type" => "response", @@ -571,13 +571,13 @@ "type" => "B", "name" => "0", "value" => "B", - "variablesReference" => 26 + "variablesReference" => 14 ), array( "type" => "B", "name" => "1", "value" => "B", - "variablesReference" => 27 + "variablesReference" => 14 ) ] ])); @@ -589,7 +589,7 @@ $seq = sendVsCommand(array( "command" => "variables", "type" => "request", - "arguments" => array("variablesReference" => 26))); + "arguments" => array("variablesReference" => 14))); $msg = json_decode(getNextVsDebugMessage(), true); checkObjEqualRecursively($msg, array( @@ -604,7 +604,7 @@ "name" => "\$aObj", "value" => "A", "namedVariables" => 2, - "variablesReference" => 28, + "variablesReference" => 16, "presentationHint" => array( "visibility" => "public" ) @@ -628,7 +628,7 @@ // The private props should contain the base class's copy of $a, only. array( - "variablesReference" => 29, + "variablesReference" => 23, "name" => "Private props", "value" => "class A", "namedVariables" => 1, @@ -640,7 +640,7 @@ // Two constants should be visible on A, HELLOA and HELLOB, and one // on class B. array( - "variablesReference" => 30, + "variablesReference" => 24, "name" => "Class Constants", "value" => "class B", "namedVariables" => 3, @@ -650,7 +650,7 @@ ), array( - "variablesReference" => 31, + "variablesReference" => 25, "name" => "Static Props", "value" => "class B", "namedVariables" => 1, @@ -667,7 +667,7 @@ "type" => "request", "seq" => 27, "arguments" => array( - "variablesReference" => 15, + "variablesReference" => 12, "start" => 1, "count" => 1 ))); @@ -697,7 +697,7 @@ "type" => "request", "arguments" => array( - "variablesReference" => 21, + "variablesReference" => 24, "start" => 1, "count" => 2 ))); @@ -738,7 +738,7 @@ "type" => "request", "arguments" => array( - "variablesReference" => 21, + "variablesReference" => 24, "start" => 100, "count" => 1 ))); @@ -769,7 +769,7 @@ "type" => "request", "arguments" => array( - "variablesReference" => 21, + "variablesReference" => 24, "start" => 1, "count" => 100 )));