common/MessageQueue.hpp | 1 common/Unit.cpp | 9 ++------ common/Unit.hpp | 3 +- test/TileCacheTests.cpp | 15 ++++++-------- test/UnitTileCache.cpp | 7 +++--- wsd/ClientSession.cpp | 19 +++--------------- wsd/ClientSession.hpp | 13 ++++++++++++ wsd/DocumentBroker.cpp | 50 ++++++++---------------------------------------- wsd/TileCache.cpp | 46 +++++++++++++++++++++++++++----------------- wsd/TileCache.hpp | 8 +++++-- 10 files changed, 77 insertions(+), 94 deletions(-)
New commits: commit 36e9d5b376ce246e405901e584a5920a45064a18 Author: Michael Meeks <michael.me...@collabora.com> AuthorDate: Thu Feb 14 20:01:43 2019 +0100 Commit: Michael Meeks <michael.me...@collabora.com> CommitDate: Thu Feb 14 21:46:39 2019 +0100 TileCache: re-factor API to work in terms of vectors, not file references. Change-Id: Ia9d48773121ab965b79ddb16b55b5d3fdcd7fd5c diff --git a/common/MessageQueue.hpp b/common/MessageQueue.hpp index d0a7e8054..74acc7ea8 100644 --- a/common/MessageQueue.hpp +++ b/common/MessageQueue.hpp @@ -125,7 +125,6 @@ private: std::vector<Payload> _queue; mutable std::mutex _mutex; std::condition_variable _cv; - }; typedef MessageQueueBase<std::vector<char>> MessageQueue; diff --git a/common/Unit.cpp b/common/Unit.cpp index 28dba3488..6221cd1e9 100644 --- a/common/Unit.cpp +++ b/common/Unit.cpp @@ -170,16 +170,13 @@ void UnitWSD::configure(Poco::Util::LayeredConfiguration &config) } void UnitWSD::lookupTile(int part, int width, int height, int tilePosX, int tilePosY, - int tileWidth, int tileHeight, std::unique_ptr<std::fstream>& cacheFile) + int tileWidth, int tileHeight, + std::shared_ptr<std::vector<char>> &tile) { - if (cacheFile && cacheFile->is_open()) - { + if (tile) onTileCacheHit(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight); - } else - { onTileCacheMiss(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight); - } } UnitKit::UnitKit() diff --git a/common/Unit.hpp b/common/Unit.hpp index 4d018b80b..2c390ad94 100644 --- a/common/Unit.hpp +++ b/common/Unit.hpp @@ -214,7 +214,8 @@ public: // ---------------- TileCache hooks ---------------- /// Called before the lookupTile call returns. Should always be called to fire events. virtual void lookupTile(int part, int width, int height, int tilePosX, int tilePosY, - int tileWidth, int tileHeight, std::unique_ptr<std::fstream>& cacheFile); + int tileWidth, int tileHeight, + std::shared_ptr<std::vector<char>> &tile); // ---------------- DocumentBroker hooks ---------------- virtual bool filterLoad(const std::string& /* sessionId */, diff --git a/test/TileCacheTests.cpp b/test/TileCacheTests.cpp index 3b1954a8b..982d0351c 100644 --- a/test/TileCacheTests.cpp +++ b/test/TileCacheTests.cpp @@ -195,8 +195,8 @@ void TileCacheTests::testSimple() TileDesc tile(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight, -1, 0, -1, false); // No Cache - std::unique_ptr<std::fstream> file = tc.lookupTile(tile); - CPPUNIT_ASSERT_MESSAGE("found tile when none was expected", !file); + TileCache::Tile tileData = tc.lookupTile(tile); + CPPUNIT_ASSERT_MESSAGE("found tile when none was expected", !tileData); // Cache Tile const int size = 1024; @@ -204,17 +204,16 @@ void TileCacheTests::testSimple() tc.saveTileAndNotify(tile, data.data(), size); // Find Tile - file = tc.lookupTile(tile); - CPPUNIT_ASSERT_MESSAGE("tile not found when expected", file && file->is_open()); - const std::vector<char> tileData = readDataFromFile(file); - CPPUNIT_ASSERT_MESSAGE("cached tile corrupted", data == tileData); + tileData = tc.lookupTile(tile); + CPPUNIT_ASSERT_MESSAGE("tile not found when expected", tileData); + CPPUNIT_ASSERT_MESSAGE("cached tile corrupted", data == *tileData); // Invalidate Tiles tc.invalidateTiles("invalidatetiles: EMPTY"); // No Cache - file = tc.lookupTile(tile); - CPPUNIT_ASSERT_MESSAGE("found tile when none was expected", !file); + tileData = tc.lookupTile(tile); + CPPUNIT_ASSERT_MESSAGE("found tile when none was expected", !tileData); } void TileCacheTests::testSimpleCombine() diff --git a/test/UnitTileCache.cpp b/test/UnitTileCache.cpp index eba7015e9..e2ee15a4d 100644 --- a/test/UnitTileCache.cpp +++ b/test/UnitTileCache.cpp @@ -34,13 +34,14 @@ public: } virtual void lookupTile(int part, int width, int height, int tilePosX, int tilePosY, - int tileWidth, int tileHeight, std::unique_ptr<std::fstream>& cacheFile) + int tileWidth, int tileHeight, + std::shared_ptr<std::vector<char>> &tile) { // Call base to fire events. - UnitWSD::lookupTile(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight, cacheFile); + UnitWSD::lookupTile(part, width, height, tilePosX, tilePosY, tileWidth, tileHeight, tile); // Fail the lookup to force subscription and rendering. - cacheFile.reset(); + tile.reset(); // FIXME: push through to the right place to exercise this. exitTest(TestResult::Ok); diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp index 3a6bf59dd..49623636a 100644 --- a/wsd/ClientSession.cpp +++ b/wsd/ClientSession.cpp @@ -504,24 +504,13 @@ bool ClientSession::sendFontRendering(const char *buffer, int length, const std: } getTokenString(tokens[2], "char", text); - const std::string response = "renderfont: " + Poco::cat(std::string(" "), tokens.begin() + 1, tokens.end()) + "\n"; - std::vector<char> output; - output.resize(response.size()); - std::memcpy(output.data(), response.data(), response.size()); - std::unique_ptr<std::fstream> cachedRendering = docBroker->tileCache().lookupCachedFile(font+text, "font"); - if (cachedRendering && cachedRendering->is_open()) + TileCache::Tile cachedTile = docBroker->tileCache().lookupCachedTile(font+text, "font"); + if (cachedTile) { - cachedRendering->seekg(0, std::ios_base::end); - size_t pos = output.size(); - std::streamsize size = cachedRendering->tellg(); - output.resize(pos + size); - cachedRendering->seekg(0, std::ios_base::beg); - cachedRendering->read(output.data() + pos, size); - cachedRendering->close(); - - return sendBinaryFrame(output.data(), output.size()); + const std::string response = "renderfont: " + Poco::cat(std::string(" "), tokens.begin() + 1, tokens.end()) + "\n"; + return sendTile(response, cachedTile); } return forwardToChild(std::string(buffer, length), docBroker); diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp index 948e1b202..827a0f857 100644 --- a/wsd/ClientSession.hpp +++ b/wsd/ClientSession.hpp @@ -65,6 +65,18 @@ public: return true; } + bool sendTile(const std::string &header, const TileCache::Tile &tile) + { + // FIXME: performance - optimize away this copy ... + std::vector<char> output; + + output.resize(header.size() + tile->size()); + std::memcpy(output.data(), header.data(), header.size()); + std::memcpy(output.data() + header.size(), tile->data(), tile->size()); + + return sendBinaryFrame(output.data(), output.size()); + } + bool sendTextFrame(const char* buffer, const int length) override { auto payload = std::make_shared<Message>(buffer, length, Message::Dir::Out); @@ -72,6 +84,7 @@ public: return true; } + void enqueueSendMessage(const std::shared_ptr<Message>& data); /// Set the save-as socket which is used to send convert-to results. diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index ae071e8ac..3cce77e62 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -1334,7 +1334,7 @@ void DocumentBroker::handleTileRequest(TileDesc& tile, const std::string tileMsg = tile.serialize(); LOG_TRC("Tile request for " << tileMsg); - std::unique_ptr<std::fstream> cachedTile = _tileCache->lookupTile(tile); + TileCache::Tile cachedTile = _tileCache->lookupTile(tile); if (cachedTile) { #if ENABLE_DEBUG @@ -1342,22 +1342,7 @@ void DocumentBroker::handleTileRequest(TileDesc& tile, #else const std::string response = tile.serialize("tile:") + '\n'; #endif - - std::vector<char> output; - output.reserve(static_cast<size_t>(4) * tile.getWidth() * tile.getHeight()); - output.resize(response.size()); - std::memcpy(output.data(), response.data(), response.size()); - - assert(cachedTile->is_open()); - cachedTile->seekg(0, std::ios_base::end); - const size_t pos = output.size(); - std::streamsize size = cachedTile->tellg(); - output.resize(pos + size); - cachedTile->seekg(0, std::ios_base::beg); - cachedTile->read(output.data() + pos, size); - cachedTile->close(); - - session->sendBinaryFrame(output.data(), output.size()); + session->sendTile(response, cachedTile); return; } @@ -1388,15 +1373,13 @@ void DocumentBroker::handleTileCombinedRequest(TileCombined& tileCombined, LOG_TRC("TileCombined request for " << tileCombined.serialize()); - // Check which newly requested tiles needs rendering. + // Check which newly requested tiles need rendering. std::vector<TileDesc> tilesNeedsRendering; for (auto& tile : tileCombined.getTiles()) { tile.setVersion(++_tileVersion); - std::unique_ptr<std::fstream> cachedTile = _tileCache->lookupTile(tile); - if(cachedTile) - cachedTile->close(); - else + TileCache::Tile cachedTile = _tileCache->lookupTile(tile); + if(!cachedTile) { // Not cached, needs rendering. tilesNeedsRendering.push_back(tile); @@ -1491,8 +1474,8 @@ void DocumentBroker::sendRequestedTiles(const std::shared_ptr<ClientSession>& se // Drop tiles which we are waiting for too long session->removeOutdatedTilesOnFly(); - // All tiles were processed on client side what we sent last time, so we can send a new banch of tiles - // which was invalidated / requested in the meantime + // All tiles were processed on client side that we sent last time, so we can send + // a new banch of tiles which was invalidated / requested in the meantime std::deque<TileDesc>& requestedTiles = session->getRequestedTiles(); if (!requestedTiles.empty()) { @@ -1517,7 +1500,7 @@ void DocumentBroker::sendRequestedTiles(const std::shared_ptr<ClientSession>& se } // Satisfy as many tiles from the cache. - std::unique_ptr<std::fstream> cachedTile = _tileCache->lookupTile(tile); + TileCache::Tile cachedTile = _tileCache->lookupTile(tile); if (cachedTile) { //TODO: Combine the response to reduce latency. @@ -1526,22 +1509,7 @@ void DocumentBroker::sendRequestedTiles(const std::shared_ptr<ClientSession>& se #else const std::string response = tile.serialize("tile:") + "\n"; #endif - - std::vector<char> output; - output.reserve(static_cast<size_t>(4) * tile.getWidth() * tile.getHeight()); - output.resize(response.size()); - std::memcpy(output.data(), response.data(), response.size()); - - assert(cachedTile->is_open()); - cachedTile->seekg(0, std::ios_base::end); - const auto pos = output.size(); - std::streamsize size = cachedTile->tellg(); - output.resize(pos + size); - cachedTile->seekg(0, std::ios_base::beg); - cachedTile->read(output.data() + pos, size); - cachedTile->close(); - - session->sendBinaryFrame(output.data(), output.size()); + session->sendTile(response, cachedTile); } else { diff --git a/wsd/TileCache.cpp b/wsd/TileCache.cpp index 65b1c5a41..b709bde2c 100644 --- a/wsd/TileCache.cpp +++ b/wsd/TileCache.cpp @@ -44,6 +44,27 @@ using Poco::File; using Poco::StringTokenizer; using Poco::Timestamp; +namespace { + TileCache::Tile loadTile(const std::string &fileName) + { + TileCache::Tile ret; + + std::unique_ptr<std::fstream> result(new std::fstream(fileName, std::ios::in)); + if (result && result->is_open()) + { + LOG_TRC("Found cache tile: " << fileName); + + result->seekg(0, std::ios_base::end); + std::streamsize size = result->tellg(); + ret = std::make_shared<std::vector<char>>(size); + result->seekg(0, std::ios_base::beg); + result->read(ret->data(), size); + result->close(); + } + return ret; + } +} + TileCache::TileCache(const std::string& docURL, const Timestamp& modifiedTime, const std::string& cacheDir, @@ -164,25 +185,19 @@ int TileCache::getTileBeingRenderedVersion(const TileDesc& tile) return 0; } -std::unique_ptr<std::fstream> TileCache::lookupTile(const TileDesc& tile) +TileCache::Tile TileCache::lookupTile(const TileDesc& tile) { if (!_tileCachePersistent) return nullptr; const std::string fileName = _cacheDir + "/" + cacheFileName(tile); + TileCache::Tile ret = loadTile(fileName); - std::unique_ptr<std::fstream> result(new std::fstream(fileName, std::ios::in)); UnitWSD::get().lookupTile(tile.getPart(), tile.getWidth(), tile.getHeight(), tile.getTilePosX(), tile.getTilePosY(), - tile.getTileWidth(), tile.getTileHeight(), result); + tile.getTileWidth(), tile.getTileHeight(), ret); - if (result && result->is_open()) - { - LOG_TRC("Found cache tile: " << fileName); - return result; - } - - return nullptr; + return ret; } void TileCache::saveTileAndNotify(const TileDesc& tile, const char *data, const size_t size) @@ -337,19 +352,16 @@ void TileCache::saveRendering(const std::string& name, const std::string& dir, c FileUtil::saveDataToFileSafely(fileName, data, size); } -std::unique_ptr<std::fstream> TileCache::lookupCachedFile(const std::string& name, const std::string& dir) +TileCache::Tile TileCache::lookupCachedTile(const std::string& name, const std::string& dir) { const std::string dirName = _cacheDir + "/" + dir; const std::string fileName = dirName + "/" + name; File directory(dirName); if (directory.exists() && directory.isDirectory() && File(fileName).exists()) - { - std::unique_ptr<std::fstream> result(new std::fstream(fileName, std::ios::in)); - return result; - } - - return nullptr; + return loadTile(fileName); + else + return Tile(); } void TileCache::invalidateTiles(int part, int x, int y, int width, int height) diff --git a/wsd/TileCache.hpp b/wsd/TileCache.hpp index db922691d..1a9ba0a26 100644 --- a/wsd/TileCache.hpp +++ b/wsd/TileCache.hpp @@ -31,6 +31,8 @@ class TileCache std::shared_ptr<TileBeingRendered> findTileBeingRendered(const TileDesc& tile); public: + typedef std::shared_ptr<std::vector<char>> Tile; + /// When the docURL is a non-file:// url, the timestamp has to be provided by the caller. /// For file:// url's, it's ignored. /// When it is missing for non-file:// url, it is assumed the document must be read, and no cached value used. @@ -54,7 +56,8 @@ public: /// Cancels all tile requests by the given subscriber. std::string cancelTiles(const std::shared_ptr<ClientSession>& subscriber); - std::unique_ptr<std::fstream> lookupTile(const TileDesc& tile); + /// Find the tile with this description + Tile lookupTile(const TileDesc& tile); void saveTileAndNotify(const TileDesc& tile, const char* data, const size_t size); @@ -73,7 +76,8 @@ public: // The dir parameter should be the type of rendering, like "font", "style", etc void saveRendering(const std::string& name, const std::string& dir, const char* data, size_t size); - std::unique_ptr<std::fstream> lookupCachedFile(const std::string& name, const std::string& dir); + /// Return the tile data if we have it, or nothing. + Tile lookupCachedTile(const std::string& name, const std::string& dir); // The tiles parameter is an invalidatetiles: message as sent by the child process void invalidateTiles(const std::string& tiles); _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits