kit/Kit.cpp | 33 +++++++++++++++------------------ wsd/DocumentBroker.cpp | 1 + wsd/TileCache.cpp | 25 +++++++++++++++++-------- wsd/TileCache.hpp | 11 +++++------ 4 files changed, 38 insertions(+), 32 deletions(-)
New commits: commit d43589c34355694b0ffbc3b129fcaba22bcdbb22 Author: Michael Meeks <michael.me...@collabora.com> Date: Thu Jun 22 13:58:50 2017 +0100 Replace now un-used locking with thread affinity assertions. Change-Id: I8c08d1618404740e9dc1d5ff2cb7d9d460ca2be5 diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index 2e37ccf7..f1785de5 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -558,6 +558,7 @@ bool DocumentBroker::load(const std::shared_ptr<ClientSession>& session, const s // Use the local temp file's timestamp. _lastFileModifiedTime = Poco::File(_storage->getRootFilePath()).getLastModified(); _tileCache.reset(new TileCache(_storage->getUri(), _lastFileModifiedTime, _cacheRoot)); + _tileCache->setThreadOwner(std::this_thread::get_id()); } LOOLWSD::dumpNewSessionTrace(getJailId(), sessionId, _uriOrig, _storage->getRootFilePath()); diff --git a/wsd/TileCache.cpp b/wsd/TileCache.cpp index ff30710f..ab3070d8 100644 --- a/wsd/TileCache.cpp +++ b/wsd/TileCache.cpp @@ -17,7 +17,6 @@ #include <fstream> #include <iostream> #include <memory> -#include <mutex> #include <sstream> #include <string> #include <vector> @@ -73,6 +72,7 @@ TileCache::TileCache(const std::string& docURL, TileCache::~TileCache() { + _owner = std::thread::id(0); LOG_INF("~TileCache dtor for uri [" << _docURL << "]."); } @@ -114,7 +114,7 @@ std::shared_ptr<TileCache::TileBeingRendered> TileCache::findTileBeingRendered(c { const std::string cachedName = cacheFileName(tileDesc); - Util::assertIsLocked(_tilesBeingRenderedMutex); + assertCorrectThread(); const auto tile = _tilesBeingRendered.find(cachedName); return tile != _tilesBeingRendered.end() ? tile->second : nullptr; @@ -124,7 +124,7 @@ void TileCache::forgetTileBeingRendered(const TileDesc& tile) { const std::string cachedName = cacheFileName(tile); - Util::assertIsLocked(_tilesBeingRenderedMutex); + assertCorrectThread(); _tilesBeingRendered.erase(cachedName); } @@ -149,7 +149,7 @@ std::unique_ptr<std::fstream> TileCache::lookupTile(const TileDesc& tile) void TileCache::saveTileAndNotify(const TileDesc& tile, const char *data, const size_t size) { - std::unique_lock<std::mutex> lock(_tilesBeingRenderedMutex); + assertCorrectThread(); std::shared_ptr<TileBeingRendered> tileBeingRendered = findTileBeingRendered(tile); @@ -321,8 +321,7 @@ void TileCache::invalidateTiles(int part, int x, int y, int width, int height) File dir(_cacheDir); - std::unique_lock<std::mutex> lock(_cacheMutex); - std::unique_lock<std::mutex> lockSubscribers(_tilesBeingRenderedMutex); + assertCorrectThread(); if (dir.exists() && dir.isDirectory()) { @@ -446,7 +445,7 @@ void TileCache::subscribeToTileRendering(const TileDesc& tile, const std::shared oss << '(' << tile.getPart() << ',' << tile.getTilePosX() << ',' << tile.getTilePosY() << ')'; const auto name = oss.str(); - std::unique_lock<std::mutex> lock(_tilesBeingRenderedMutex); + assertCorrectThread(); std::shared_ptr<TileBeingRendered> tileBeingRendered = findTileBeingRendered(tile); @@ -493,7 +492,7 @@ std::string TileCache::cancelTiles(const std::shared_ptr<ClientSession> &subscri assert(subscriber && "cancelTiles expects valid subscriber"); LOG_TRC("Cancelling tiles for " << subscriber->getName()); - std::unique_lock<std::mutex> lock(_tilesBeingRenderedMutex); + assertCorrectThread(); const auto sub = subscriber.get(); @@ -534,4 +533,14 @@ std::string TileCache::cancelTiles(const std::shared_ptr<ClientSession> &subscri return canceltiles.empty() ? canceltiles : "canceltiles " + canceltiles; } +void TileCache::assertCorrectThread() +{ + const bool correctThread = _owner == std::thread::id(0) || std::this_thread::get_id() == _owner; + if (!correctThread) + LOG_ERR("TileCache method invoked from foreign thread. Expected: 0x" << std::hex << + _owner << " but called from 0x" << std::this_thread::get_id() << " (" << + std::dec << Util::getThreadId() << ")."); + assert (correctThread); +} + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/wsd/TileCache.hpp b/wsd/TileCache.hpp index 9f86cc72..5f8e71cf 100644 --- a/wsd/TileCache.hpp +++ b/wsd/TileCache.hpp @@ -13,7 +13,7 @@ #include <iosfwd> #include <map> #include <memory> -#include <mutex> +#include <thread> #include <string> #include <Poco/Timestamp.h> @@ -75,10 +75,11 @@ public: /// Store the timestamp to modtime.txt. void saveLastModified(const Poco::Timestamp& timestamp); - std::unique_lock<std::mutex> getTilesBeingRenderedLock() { return std::unique_lock<std::mutex>(_tilesBeingRenderedMutex); } - void forgetTileBeingRendered(const TileDesc& tile); + void setThreadOwner(const std::thread::id &id) { _owner = id; } + void assertCorrectThread(); + private: void invalidateTiles(int part, int x, int y, int width, int height); @@ -98,9 +99,7 @@ private: const std::string _cacheDir; - std::mutex _cacheMutex; - - mutable std::mutex _tilesBeingRenderedMutex; + std::thread::id _owner; std::map<std::string, std::shared_ptr<TileBeingRendered> > _tilesBeingRendered; }; commit 3f59438f629aee8eacef48058ce16ad8a5787868 Author: Michael Meeks <michael.me...@collabora.com> Date: Wed Jun 21 15:03:54 2017 +0100 Simplify renderid generation, and re-order. Change-Id: I3f934fdf16b1a9cf4ea00571616d9b5568535707 diff --git a/kit/Kit.cpp b/kit/Kit.cpp index be07bbb8..f811be5d 100644 --- a/kit/Kit.cpp +++ b/kit/Kit.cpp @@ -105,6 +105,12 @@ class Document; static std::shared_ptr<Document> document; static LokHookFunction2* initFunction = nullptr; +#if ENABLE_DEBUG +# define ADD_DEBUG_RENDERID(s) ((s)+ " renderid=" + Util::UniqueId()) +#else +# define ADD_DEBUG_RENDERID(s) (s) +#endif + namespace { #ifndef BUILDING_TESTS @@ -685,26 +691,21 @@ public: tile.setWireId(wid); + if (hash != 0 && tile.getOldWireId() == wid) + { + // The tile content is identical to what the client already has, so skip it + LOG_TRC("Match oldWireId==wid (" << wid << " for hash " << hash << "); unchanged"); + return; + } + // Send back the request with all optional parameters given in the request. - const auto tileMsg = tile.serialize("tile:"); -#if ENABLE_DEBUG - const std::string response = tileMsg + " renderid=" + Util::UniqueId() + "\n"; -#else - const std::string response = tileMsg + "\n"; -#endif + std::string response = ADD_DEBUG_RENDERID(tile.serialize("tile:")) + "\n"; std::vector<char> output; output.reserve(response.size() + pixmapDataSize); output.resize(response.size()); std::memcpy(output.data(), response.data(), response.size()); - if (hash != 0 && tile.getOldWireId() == wid) - { - // The tile content is identical to what the client already has, so skip it - LOG_TRC("Match oldWireId==wid (" << wid << " for hash " << hash << "), skipping"); - return; - } - if (!_pngCache.encodeBufferToPNG(pixmap.data(), tile.getWidth(), tile.getHeight(), output, mode, hash, wid)) { //FIXME: Return error. @@ -826,11 +827,7 @@ public: renderArea.getWidth() << ", " << renderArea.getHeight() << ") " << " took " << (elapsed/1000.) << " ms (including the paintTile)."); -#if ENABLE_DEBUG - const auto tileMsg = tileCombined.serialize("tilecombine:") + " renderid=" + Util::UniqueId() + "\n"; -#else - const auto tileMsg = tileCombined.serialize("tilecombine:") + "\n"; -#endif + const auto tileMsg = ADD_DEBUG_RENDERID(tileCombined.serialize("tilecombine:")) + "\n"; LOG_TRC("Sending back painted tiles for " << tileMsg); std::vector<char> response; _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits