From 92846305f58bc4844db4c45610083f0788cfbde7 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Fri, 13 Oct 2023 14:43:34 -0700 Subject: [PATCH] matdbg: change from websocket to GET (#7263) - Use a hanging-GET approach to reduce dependency on websockets. - Also add mutex to protect access to MaterialRecords, which is written to/read from from multiple threads. --- libs/matdbg/include/matdbg/DebugServer.h | 11 +-- libs/matdbg/src/ApiHandler.cpp | 42 ++++++++++- libs/matdbg/src/ApiHandler.h | 19 ++++- libs/matdbg/src/DebugServer.cpp | 56 ++++----------- libs/matdbg/web/script.js | 89 +++++++++++++----------- 5 files changed, 123 insertions(+), 94 deletions(-) diff --git a/libs/matdbg/include/matdbg/DebugServer.h b/libs/matdbg/include/matdbg/DebugServer.h index 48a0b25e0fa..4340001d661 100644 --- a/libs/matdbg/include/matdbg/DebugServer.h +++ b/libs/matdbg/include/matdbg/DebugServer.h @@ -24,6 +24,7 @@ #include #include +#include class CivetServer; @@ -43,9 +44,8 @@ struct MaterialRecord { /** * Server-side material debugger. * - * This class manages an HTTP server and a WebSockets server that listen on a secondary thread. It - * receives material packages from the Filament C++ engine or from a standalone tool such as - * matinfo. + * This class manages an HTTP server. It receives material packages from the Filament C++ engine or + * from a standalone tool such as matinfo. */ class DebugServer { public: @@ -99,7 +99,10 @@ class DebugServer { const backend::Backend mBackend; CivetServer* mServer; + tsl::robin_map mMaterialRecords; + mutable utils::Mutex mMaterialRecordsMutex; + utils::CString mHtml; utils::CString mJavascript; utils::CString mCss; @@ -112,11 +115,9 @@ class DebugServer { class FileRequestHandler* mFileHandler = nullptr; class ApiHandler* mApiHandler = nullptr; - class WebSocketHandler* mWebSocketHandler = nullptr; friend class FileRequestHandler; friend class ApiHandler; - friend class WebSocketHandler; }; } // namespace filament::matdbg diff --git a/libs/matdbg/src/ApiHandler.cpp b/libs/matdbg/src/ApiHandler.cpp index 42cb9eb2de3..6a7d801e121 100644 --- a/libs/matdbg/src/ApiHandler.cpp +++ b/libs/matdbg/src/ApiHandler.cpp @@ -41,14 +41,14 @@ namespace { auto const& kSuccessHeader = DebugServer::kSuccessHeader; auto const& kErrorHeader = DebugServer::kErrorHeader; -void spirvToAsm(struct mg_connection *conn, uint32_t const* spirv, size_t size) { +void spirvToAsm(struct mg_connection* conn, uint32_t const* spirv, size_t size) { auto spirvDisassembly = ShaderExtractor::spirvToText(spirv, size / 4); mg_printf(conn, kSuccessHeader.data(), "application/txt"); mg_write(conn, spirvDisassembly.c_str(), spirvDisassembly.size()); } -void spirvToGlsl(ShaderModel shaderModel, struct mg_connection *conn, - uint32_t const* spirv, size_t size) { +void spirvToGlsl(ShaderModel shaderModel, struct mg_connection* conn, uint32_t const* spirv, + size_t size) { auto glsl = ShaderExtractor::spirvToGLSL(shaderModel, spirv, size / 4); mg_printf(conn, kSuccessHeader.data(), "application/txt"); mg_printf(conn, glsl.c_str(), glsl.size()); @@ -212,6 +212,33 @@ bool ApiHandler::handleGetApiShader(struct mg_connection* conn, return error(__LINE__, uri); } +void ApiHandler::addMaterial(MaterialRecord const* material) { + std::unique_lock lock(mStatusMutex); + snprintf(statusMaterialId, sizeof(statusMaterialId), "%8.8x", material->key); + mStatusCondition.notify_all(); +} + +bool ApiHandler::handleGetStatus(struct mg_connection* conn, + struct mg_request_info const* request) { + char const* qstr = request->query_string; + if (qstr && strcmp(qstr, "firstTime") == 0) { + mg_printf(conn, kSuccessHeader.data(), "application/txt"); + mg_write(conn, "0", 1); + return true; + } + + { + std::unique_lock lock(mStatusMutex); + uint64_t const currentStatusCount = mCurrentStatus; + mStatusCondition.wait(lock, + [this, currentStatusCount] { return currentStatusCount < mCurrentStatus; }); + + mg_printf(conn, kSuccessHeader.data(), "application/txt"); + mg_write(conn, statusMaterialId, 8); + } + return true; +} + bool ApiHandler::handlePost(CivetServer* server, struct mg_connection* conn) { struct mg_request_info const* request = mg_get_request_info(conn); std::string const& uri = request->local_uri; @@ -264,6 +291,9 @@ bool ApiHandler::handleGet(CivetServer* server, struct mg_connection* conn) { if (uri == "/api/active") { mServer->updateActiveVariants(); + + // Careful not to lock the above line. + std::unique_lock lock(mServer->mMaterialRecordsMutex); mg_printf(conn, kSuccessHeader.data(), "application/json"); mg_printf(conn, "{"); @@ -294,6 +324,7 @@ bool ApiHandler::handleGet(CivetServer* server, struct mg_connection* conn) { } if (uri == "/api/matids") { + std::unique_lock lock(mServer->mMaterialRecordsMutex); mg_printf(conn, kSuccessHeader.data(), "application/json"); mg_printf(conn, "["); int index = 0; @@ -306,6 +337,7 @@ bool ApiHandler::handleGet(CivetServer* server, struct mg_connection* conn) { } if (uri == "/api/materials") { + std::unique_lock lock(mServer->mMaterialRecordsMutex); mg_printf(conn, kSuccessHeader.data(), "application/json"); mg_printf(conn, "["); int index = 0; @@ -353,6 +385,10 @@ bool ApiHandler::handleGet(CivetServer* server, struct mg_connection* conn) { return handleGetApiShader(conn, request); } + if (uri.find("/api/status") == 0) { + return handleGetStatus(conn, request); + } + return error(__LINE__, uri); } diff --git a/libs/matdbg/src/ApiHandler.h b/libs/matdbg/src/ApiHandler.h index f76169e7627..27464d56267 100644 --- a/libs/matdbg/src/ApiHandler.h +++ b/libs/matdbg/src/ApiHandler.h @@ -17,6 +17,8 @@ #ifndef MATDBG_APIHANDLER_H #define MATDBG_APIHANDLER_H +#include + #include namespace filament::matdbg { @@ -31,23 +33,34 @@ struct MaterialRecord; // GET /api/material?matid={id} // GET /api/shader?matid={id}&type=[glsl|spirv]&[glindex|vkindex|metalindex]={index} // GET /api/active +// GET /api/status // POST /api/edit // class ApiHandler : public CivetHandler { public: explicit ApiHandler(DebugServer* server) : mServer(server) {} + ~ApiHandler() = default; bool handleGet(CivetServer* server, struct mg_connection* conn); - bool handlePost(CivetServer* server, struct mg_connection* conn); - - ~ApiHandler() = default; + void addMaterial(MaterialRecord const* material); private: bool handleGetApiShader(struct mg_connection* conn, struct mg_request_info const* request); + bool handleGetStatus(struct mg_connection* conn, struct mg_request_info const* request); MaterialRecord const* getMaterialRecord(struct mg_request_info const* request); + DebugServer* mServer; + + utils::Mutex mStatusMutex; + std::condition_variable mStatusCondition; + char statusMaterialId[9] = {}; + + // This variable is to implement a *hanging* effect for /api/status. The call to /api/status + // will always block until statusMaterialId is updated again. The client is expected to keep + // calling /api/status (a constant "pull" to simulate a push). + std::atomic_uint64_t mCurrentStatus = 0; }; } // filament::matdbg diff --git a/libs/matdbg/src/DebugServer.cpp b/libs/matdbg/src/DebugServer.cpp index 62de97fb29d..99e8fe52a33 100644 --- a/libs/matdbg/src/DebugServer.cpp +++ b/libs/matdbg/src/DebugServer.cpp @@ -116,42 +116,6 @@ class FileRequestHandler : public CivetHandler { DebugServer* mServer; }; -class WebSocketHandler : public CivetWebSocketHandler { -public: - WebSocketHandler(DebugServer* server) : mServer(server) {} - - bool handleConnection(CivetServer *server, const struct mg_connection *conn) override { - return true; - } - - void handleReadyState(CivetServer *server, struct mg_connection *conn) override { - mConnections.insert(conn); - } - - bool handleData(CivetServer *server, struct mg_connection *conn, int bits, char *data, - size_t size) override { - return false; - } - - void handleClose(CivetServer *server, const struct mg_connection *conn) override { - struct mg_connection *key = const_cast(conn); - mConnections.erase(key); - } - - // Notify all JavaScript clients that a new material package has been loaded. - void addMaterial(MaterialRecord const& material) { - for (auto connection : mConnections) { - char matid[9] = {}; - snprintf(matid, sizeof(matid), "%8.8x", material.key); - mg_websocket_write(connection, MG_WEBSOCKET_OPCODE_TEXT, matid, 8); - } - } - -private: - DebugServer* mServer; - tsl::robin_set mConnections; -}; - DebugServer::DebugServer(Backend backend, int port) : mBackend(backend) { #if !SERVE_FROM_SOURCE_TREE mHtml = CString((const char*) MATDBG_RESOURCES_INDEX_DATA, MATDBG_RESOURCES_INDEX_SIZE - 1); @@ -182,11 +146,9 @@ DebugServer::DebugServer(Backend backend, int port) : mBackend(backend) { mFileHandler = new FileRequestHandler(this); mApiHandler = new ApiHandler(this); - mWebSocketHandler = new WebSocketHandler(this); mServer->addHandler("/api", mApiHandler); mServer->addHandler("", mFileHandler); - mServer->addWebSocketHandler("", mWebSocketHandler); slog.i << "DebugServer listening at http://localhost:" << port << io::endl; filamat::GLSLTools::init(); @@ -194,12 +156,17 @@ DebugServer::DebugServer(Backend backend, int port) : mBackend(backend) { DebugServer::~DebugServer() { filamat::GLSLTools::shutdown(); - for (auto& pair : mMaterialRecords) { - delete [] pair.second.package; - } + + mServer->close(); + delete mFileHandler; delete mApiHandler; delete mServer; + + std::unique_lock lock(mMaterialRecordsMutex); + for (auto& pair : mMaterialRecords) { + delete [] pair.second.package; + } } MaterialKey @@ -218,23 +185,27 @@ DebugServer::addMaterial(const CString& name, const void* data, size_t size, voi uint8_t* package = new uint8_t[size]; memcpy(package, data, size); + std::unique_lock lock(mMaterialRecordsMutex); MaterialRecord info = {userdata, package, size, name, key}; mMaterialRecords.insert({key, info}); - mWebSocketHandler->addMaterial(info); + mApiHandler->addMaterial(&info); return key; } void DebugServer::removeMaterial(MaterialKey key) { + std::unique_lock lock(mMaterialRecordsMutex); mMaterialRecords.erase(key); } const MaterialRecord* DebugServer::getRecord(const MaterialKey& key) const { + std::unique_lock lock(mMaterialRecordsMutex); const auto& iter = mMaterialRecords.find(key); return iter == mMaterialRecords.end() ? nullptr : &iter->second; } void DebugServer::updateActiveVariants() { if (mQueryCallback) { + std::unique_lock lock(mMaterialRecordsMutex); auto curr = mMaterialRecords.begin(); auto end = mMaterialRecords.end(); while (curr != end) { @@ -253,6 +224,7 @@ bool DebugServer::handleEditCommand(const MaterialKey& key, backend::Backend api return false; }; + std::unique_lock lock(mMaterialRecordsMutex); if (mMaterialRecords.find(key) == mMaterialRecords.end()) { return error(__LINE__); } diff --git a/libs/matdbg/web/script.js b/libs/matdbg/web/script.js index 5fe11b7e5c8..9477ab05b7a 100644 --- a/libs/matdbg/web/script.js +++ b/libs/matdbg/web/script.js @@ -25,14 +25,14 @@ const shaderSource = document.getElementById("shader-source"); const matDetailTemplate = document.getElementById("material-detail-template"); const matListTemplate = document.getElementById("material-list-template"); +const STATUS_LOOP_TIMEOUT = 3000; + const gMaterialDatabase = {}; -let gSocket = null; let gEditor = null; let gCurrentMaterial = "00000000"; let gCurrentLanguage = "glsl"; let gCurrentShader = { matid: "00000000", glindex: 0 }; -let gCurrentSocketId = 0; let gEditorIsLoading = false; require.config({ paths: { "vs": `${kMonacoBaseUrl}vs` }}); @@ -166,16 +166,7 @@ function fetchMaterial(matid) { } function queryActiveShaders() { - if (!gSocket) { - for (matid in gMaterialDatabase) { - const material = gMaterialDatabase[matid]; - material.active = false; - for (const shader of material.opengl) shader.active = false; - for (const shader of material.vulkan) shader.active = false; - for (const shader of material.metal) shader.active = false; - } - renderMaterialList(); - renderMaterialDetail(); + if (!isConnected()) { return; } fetch("api/active").then(function(response) { @@ -204,38 +195,54 @@ function queryActiveShaders() { }); } -function startSocket() { - const url = new URL(document.URL) - const ws = new WebSocket(`ws://${url.host}`); - - // When a new server has come online, ask it what materials it has. - ws.addEventListener("open", () => { - footer.innerText = `connection ${gCurrentSocketId}`; - gCurrentSocketId++; - - fetch("api/matids").then(function(response) { - return response.json(); - }).then(function(matInfo) { - for (matid of matInfo) { - if (!(matid in gMaterialDatabase)) { - fetchMaterial(matid); - } - } - }); - }); +function isConnected() { + return footer.innerText == 'connected'; +} - ws.addEventListener("close", (e) => { - footer.innerText = "no connection"; - gSocket = null; - setTimeout(() => startSocket(), 3000); +function onConnected() { + footer.innerText = 'connected'; + fetch("api/matids").then(function(response) { + return response.json(); + }).then(function(matInfo) { + for (matid of matInfo) { + if (!(matid in gMaterialDatabase)) { + fetchMaterial(matid); + } + } }); +} - ws.addEventListener("message", event => { - const matid = event.data; - fetchMaterial(matid); - }); +function onDisconnected() { + footer.innerText = 'not connected'; + for (matid in gMaterialDatabase) { + const material = gMaterialDatabase[matid]; + material.active = false; + for (const shader of material.opengl) shader.active = false; + for (const shader of material.vulkan) shader.active = false; + for (const shader of material.metal) shader.active = false; + } + renderMaterialList(); + renderMaterialDetail(); +} - gSocket = ws; +function statusLoop() { + // This is a hanging get except for when transition from disconnected to connected, which + // should return immediately. + fetch("api/status" + (isConnected() ? '' : '?firstTime')) + .then(async (response) => { + const matid = await response.text(); + // A first-time request returned successfully + if (matid === '0') { + onConnected(); + } else { + fetchMaterial(matid); + } + statusLoop(); + }) + .catch(err => { + onDisconnected(); + setTimeout(statusLoop, STATUS_LOOP_TIMEOUT) + }); } function fetchMaterials() { @@ -498,7 +505,7 @@ function init() { Mustache.parse(matDetailTemplate.innerHTML); Mustache.parse(matListTemplate.innerHTML); - startSocket(); + statusLoop(); // Poll for active shaders once every second. // Take care not to poll more frequently than the frame rate. Active variants are determined