loolwsd/LOOLKit.cpp | 2 -- 1 file changed, 2 deletions(-) New commits: commit 335611b0ff2bcbe2341d9718f49c8c9139c5c4b5 Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk> Date: Sun May 29 18:31:56 2016 -0400
loolwsd: don't unlock too soon Since rendering moved to centralized WSD<->Kit processing, it runs on a different thread than those processing the communication between ClientSession and ChildSession. This introduces a new race between events and tile rendering. The shared ChildSession lock prevents this race such that no events are processed while a tile is rendered and, more importantly, response is prepared and sent back. That preparation could be lengthy due to png compression. The typical race happens when two keystrokes are entered in quick succession such that the same tile is invalidated while it's rendered. If the invalidation is processed in parallel to rendering, it will find no cached image to remove. It will reach the client, who will request a new tile. Meanwhile, the first tile is rendered and cached. By the time the second request arrives we have a cache hit, albeit on an outdated tile, missing the second character. By locking until the tile response is sent we ensure that the invalidate event will follow it, and by then the image will have been cached. The invalidation then removes the cached image and the second tile request is forced to place a new tile render request. There is some inefficiency, it would seem, but that is not really true, as Core is really sequential anyway and we shouldn't process events in parallel in the first place. Change-Id: Id8170a41a7e69bca6ac8b520029b7cdb2d96c880 Reviewed-on: https://gerrit.libreoffice.org/25662 Reviewed-by: Ashod Nakashian <ashnak...@gmail.com> Tested-by: Ashod Nakashian <ashnak...@gmail.com> diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp index 626435a..b462076 100644 --- a/loolwsd/LOOLKit.cpp +++ b/loolwsd/LOOLKit.cpp @@ -597,7 +597,6 @@ public: Log::trace() << "paintTile at (" << tile.getPart() << ',' << tile.getTilePosX() << ',' << tile.getTilePosY() << ") rendered in " << (timestamp.elapsed()/1000.) << " ms" << Log::end; const auto mode = static_cast<LibreOfficeKitTileMode>(_loKitDocument->getTileMode()); - lock.unlock(); if (!png::encodeBufferToPNG(pixmap.data(), tile.getWidth(), tile.getHeight(), output, mode)) { @@ -667,7 +666,6 @@ public: << " (" << renderArea.getWidth() << ", " << renderArea.getHeight() << ") rendered in " << double(timestamp.elapsed())/1000 << " ms." << Log::end; const auto mode = static_cast<LibreOfficeKitTileMode>(_loKitDocument->getTileMode()); - lock.unlock(); std::vector<char> output; output.reserve(pixmapWidth * pixmapHeight * 4); _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits