Review Request: Update KFileItem and KFileMetaPropsPlugin to only request limited metaData (avoid reading all the file and blocking the UI)

[Darío Andrés]

> On 2010-03-24 20:45:48, Peter Penz wrote:
> > From my point of view this patch should be committed and backported to KDE SC 4.4.x. 
> > 
> > > Things to discuss:
> > > - If we choose my implementation we should probably state it on KFileInfo apidox
> > > ("metaInfo defaults to X, Z. Query your own KFileMetaInfo if you need more")
> > 
> > Yes, this should be documented well.
> > 
> > > - Should we preserve the KFileMetaInfo::What flag (and implement it on a future
> > > using Strigi); or should the flag be removed and another flag added
> > > (something like ScanAllTheFile, ScanTheFirstKBs) ?
> > 
> > I think this should be answered by Jos van den Oever. From my point of view (as application developer) I'd never chose the option ScanAllTheFile, even if it is done in a custom thread. Just providing a maximum file size value like done for preview-jobs might be an option and conform the way Strigi works internally.
> > 
> > > - Should we default to reading only the first 64kb of every file (as the previous
> > > approach) and introduce a new value of the flag "EverythingAllTheFile"
> > > (seems redundant) for Nick to use on Lokalize ?
> > 
> > I'd prefer making the flags obsolete and introduce a max. file size value with a default value of 64 KB (and probably -1 as unlimited).
> 
> Nick Shaforostoff wrote:
>     +1 for backporting it to KDE SC 4.4.x.
>     
>     about threading: as I call KFileMetaInfo in non-GUI thread, I found out that I must pause it for the time when FileOpen/FileSave Dialog is open.
>     
>     Also I implemented in trunk a custom metainfo-caching using sqlite database: http://websvn.kde.org/trunk/KDE/kdesdk/lokalize/src/project/projectmodel.cpp?r1=1037354&r2=1094015

I just saw that we could simple use the KFileItem::metaInfo() "what" flag  (declared but currently unused). This flag defaults to KFileMetaInfo::Fastest

I guess that using that parameter would be better than hardcode other values in the function itself..
 
Also, Jos wrote:

"
You can tell Strigi what information to retrieve by stating the names of the
fields from the ontologies you would like in AnalyzerConfiguration. A mapping
from KFileMetaInfo::What to collections of these field names is possible.
"

and about setting a limit filesize:

"
There is currently no generic way to know how much of a file must be read to
retrieve the value of a particular field.
"


- Darío


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3325/#review4669
-----------------------------------------------------------


On 2010-03-24 21:03:40, Darío Andrés wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3325/
> -----------------------------------------------------------
> 
> (Updated 2010-03-24 21:03:40)
> 
> 
> Review request for kdelibs, Peter Penz, Nick Shaforostoff, David Faure, and vandenoever.
> 
> 
> Summary
> -------
> 
> Backstory:
> - KFileMetaInfo::What modes are not honored because they are unimplemented with Strigi (not really important in this case)
> - KFileMetaInfo default What value is "Everything"
> - Nick Shaforostoff (Lokalize maintainer) changed the default behavior of KFileMetaInfo to read the whole file when What is "Everything" (previously, it only read the first 64kb). This 64kb limit needs to be broken for metainformation to be fetched from .po files.
> 
> So, currently the file properties dialog (and every other impl using KFileItem::metaInfo()) will read the whole content of the file , blocking the UI with big files
> 
> My patch sets both implementations to use "KFileMetaInfo::TechnicalInfo | KFileMetaInfo::ContentInfo" instead of the default Everything (in any case, as those flags are not implemented, anything different to Everything would work)
> 
> Peter already plains to solve this in the File Properties dialog with his own threading method, but this patch covers the other situations (they are complementary fixes)
> 
> Things to discuss:
> - If we choose my implementation we should probably state it on KFileInfo apidox ("metaInfo defaults to X, Z. Query your own KFileMetaInfo if you need more")
> - Should we preserve the KFileMetaInfo::What flag (and implement it on a future using Strigi); or should the flag be removed and another flag added (something like ScanAllTheFile, ScanTheFirstKBs) ?
> - Should we default to reading only the first 64kb of every file (as the previous approach) and introduce a new value of the flag "EverythingAllTheFile" (seems redundant) for Nick to use on Lokalize ?
> 
> References:
> Nick original patch: http://reviewboard.kde.org/r/2215/
> Peter threaded implementation: http://reviewboard.kde.org/r/3277/
> 
> 
> This addresses bug 216932.
>     https://bugs.kde.org/show_bug.cgi?id=216932
> 
> 
> Diffs
> -----
> 
>   svn://anonsvn.kde.org/home/kde/trunk/KDE/kdelibs/kio/kfile/kmetaprops.cpp 1105269 
>   svn://anonsvn.kde.org/home/kde/trunk/KDE/kdelibs/kio/kio/kfileitem.cpp 1105269 
> 
> Diff: http://reviewboard.kde.org/r/3325/diff
> 
> 
> Testing
> -------
> 
> Dolphin doesn't block when accessing the file properties dialog of a big (~400mb) file
> 
> 
> Thanks,
> 
> Darío
> 
>