Bug 56007 - Explorer shell extension: Thumbnail generation slow on network drives
Summary: Explorer shell extension: Thumbnail generation slow on network drives
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All Windows (All)
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:4.2.0 target:4.1.4
Keywords:
: 70224 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-10-15 15:38 UTC by Mathieu Parent
Modified: 2014-02-03 17:09 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments
Patch v1 (4.28 KB, patch)
2012-10-15 15:47 UTC, Mathieu Parent
Details
NCP capture (2.41 MB, application/x-extension-pcap)
2012-10-16 07:52 UTC, Mathieu Parent
Details
Patch v2 (4.27 KB, patch)
2012-10-16 07:57 UTC, Mathieu Parent
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mathieu Parent 2012-10-15 15:38:55 UTC
This is a Windows-specific bug.

When browsing on a slow FS, typicaly a network file system, Windows Explorer hang while loading the thumbnails.
Comment 1 Mathieu Parent 2012-10-15 15:45:17 UTC
This problem is seen on a Novell Storage Services (NSS, or ncpfs on Linux).

A network capture shows that file is read one byte at a time.

Digging further, I found that shell/source/win32/shlxthandler/thumbviewer/thumbviewer.cxx load a ZipFile (line 360) and then call GetUncompressedContent on it. This call readCentralDirectoryEntry several times and readCentralDirectoryEnd. Those two fuctions use readByte which "sread" one char at a time !
Comment 2 Mathieu Parent 2012-10-15 15:47:31 UTC
Created attachment 68583 [details]
Patch v1

Here is a proposed patch. I cannot build yet, as my build environment is Linux only.
Comment 3 Mathieu Parent 2012-10-16 07:52:52 UTC
Created attachment 68605 [details]
NCP capture

Here is a network capture showing the problem.

It seems that there is a problem in the Windows Netware Client : When 1byte is "sread", 512 are requested within the protocol. This looks like readahead (which is good), but when requesting the following byte, a new request is made (i.e the previous buffer was not cached).
Comment 4 Mathieu Parent 2012-10-16 07:57:12 UTC
Created attachment 68607 [details]
Patch v2

Patch v1 was no removing two clear().
Comment 5 Michael Stahl (CIB) 2012-12-11 17:14:48 UTC
i'm not sure if that is a bug in the zip code or in that network
file system; does the sread() not use a buffer somewhere?

but it looks like it improves the code in any case with the new
readString replacing numerous loops, and better error handling
is also nice.

there is a copy/paste error, 2 in readInt should be 4:

+    if (numBytesRead != 2)
+        throw IOException(-1);

hmm... this looks like it wants to initialize a pointer with the size value?
that cannot end well... perhaps better to create a real "string"
with a specified size and read into that, if that is possible...

+    unsigned char *tmpBuf(size);

it appears to me that your patch only works with one endian-ness,
but given that this is win32 only code and windows only works
in one endian-ness anyway that is without practical relevance.

could you please send a mail with a text like 
http://permalink.gmane.org/gmane.comp.documentfoundation.libreoffice.devel/38402
to the mailing list libreoffice@lists.freedesktop.org ?

also best to send the patch either to mailing list or to
gerrit (http://wiki.documentfoundation.org/Development/gerrit) because
developers look there far more often than at bugzilla for patches.
Comment 6 Michael Meeks 2013-10-11 16:44:34 UTC
Mathieu - it'd be great to get your changes committed here - but we need a license statement on the list first ... [ which sucks ].

I'd love to get your patch in to fix the 2nd half of this problem, but the 1st half is fixed by a commit I'll make in a second.
Comment 7 Commit Notification 2013-10-11 16:58:39 UTC
Michael Meeks committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=53138c9968e28a25a8cd6d2b5e3d31cbb3257852

fdo#56007 - fast zip directory find to accelerate Windows Explorer thumbnail.



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 8 Urmas 2013-10-11 18:02:21 UTC
*** Bug 70224 has been marked as a duplicate of this bug. ***
Comment 9 Mathieu Parent 2013-10-13 08:03:00 UTC
(In reply to comment #6)
> Mathieu - it'd be great to get your changes committed here - but we need a
> license statement on the list first ... [ which sucks ].
> 
> I'd love to get your patch in to fix the 2nd half of this problem, but the
> 1st half is fixed by a commit I'll make in a second.

Hi. Done on: http://permalink.gmane.org/gmane.comp.documentfoundation.libreoffice.devel/54246
Comment 10 Michael Meeks 2013-10-14 21:26:48 UTC
Thanks Mathieu ! I fixed up the missing ';' trailing readString, and adapted for mst's comments (thanks for the review), and pushed to master.

Looking forward to your next contribution ! :-)
Comment 11 Commit Notification 2013-10-14 21:34:05 UTC
Mathieu Parent committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=d67cd21033877c9c09d9cc4f14c2c4658e973f57

fdo#56007 - Read more bytes on Zip read (for thumbnails)



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 12 Commit Notification 2013-10-16 15:01:44 UTC
Michael Meeks committed a patch related to this issue.
It has been pushed to "libreoffice-4-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=0540be231b0a10ed97da911462874a329bd089f7&h=libreoffice-4-1

fdo#56007 - fast zip directory find to accelerate Windows Explorer thumbnail.


It will be available in LibreOffice 4.1.4.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 13 Commit Notification 2013-10-16 15:02:02 UTC
Mathieu Parent committed a patch related to this issue.
It has been pushed to "libreoffice-4-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=e09cfff67b021efd04235987f9c27cea3da5c72b&h=libreoffice-4-1

fdo#56007 - Read more bytes on Zip read (for thumbnails)


It will be available in LibreOffice 4.1.4.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 14 Matthieu 2013-10-16 15:04:17 UTC
Hello,

Is it possible to push it to libreoffice-4-0 ? And so it would be available for 4.06 version?
Comment 15 Jean-Baptiste Faure 2013-10-16 18:45:23 UTC
Perhaps a stupid question: is it possible that this bug could be related to bug 67534 since both involve the Explorer shell extension?

Best regards. JBF
Comment 16 Andras Timar 2013-10-16 19:36:23 UTC
(In reply to comment #15)
I think it is related, and this fix will solve that bug, too. If shell extension spends significantly less time on getting the metadata of the file, it will not interfere with user who double clicks on the file.
Comment 17 Matthieu 2013-10-25 09:13:13 UTC
Hello,

I confirm that it resolve also fdo#67534.

To test you must select the file to generate the explorer thumbnail and after you have to try to open the file. Without the patch it will fail.

If you open the file with DOS command by example, without selecting et generating the thumbnail it will be ok.

Is there any chance to push the commit on 4.0 branch ?
Comment 18 Michael Stahl (CIB) 2013-10-25 10:28:34 UTC
(In reply to comment #17)

> Is there any chance to push the commit on 4.0 branch ?

since 4.0.6 was released yesterday and there are no more 4.0.x
releases planned (https://wiki.documentfoundation.org/ReleasePlan/4.0)
there isn't any point to doing that.