Regarding commit b9014d21caea4d8fe98c94c1fe6158b65db5db53 by Milian (use IDocument for mimetype query when available)

[Milian Wolff]

David Nolden, 10.06.2011:
> 2011/6/10 Milian Wolff <mail at milianw.de>:
> > David Nolden, 10.06.2011:
> >> commit b9014d21caea4d8fe98c94c1fe6158b65db5db53
> >> Author: Milian Wolff <mail at milianw.de>
> >> Date:   Fri Jun 10 15:09:11 2011 +0200
> >> 
> >>     use IDocument for mimetype query when available in language
> >> controller
> >> 
> >>     this way we get proper language support when opening files without
> >> extension like e.g. <memory> or similar system headers. Furthermore
> >> there are scripting
> >>     languages that use the shebang instead of the extension to
> >> advertise their mimetype
> >> 
> >> 
> >> The idea is good, but it is not thread-safe to call documentForUrl,
> >> while languageForUrl can be called from any thread. This patch can
> >> thus lead to random crashes I think..
> > 
> > You are right, there is a *very* slim chance of a race when the doc is
> > returned, then deleted in UI-thread, and we then try to get the mimetype.
> > That's what you think, or?
> 
> I mean generally, also using the internal structures of
> DocumentController in a not thread-safe way. When a background-thread
> asks for a language support, and in the same moment the foreground
> closes/opens a new view, a crash may follow. languageForUrl is called
> very often, so the chance is not so tiny.
> 
> > Anyhow, we could still check the same stuff as further below, i.e.
> > compare current thread to qapp's thread.
> > 
> >> A better solution would be: When opening a document, force the
> >> language-controller to cache its type, or something like that.
> > 
> > Per-Url caching? Or what do you have in mind?
> > 
> > Seriously though, I really want to improve the current situation
> > regarding "dont look into files for mimetypes". I get that it's a severe
> > issue for you performance-wise, but it is also a nasty issue for
> > everyone trying to work with extension-less files.
> > 
> > Introducing a per-directory cache for extension-less files would be OK
> > for the C++ case, but breaks again for dirs containing shebang-ed
> > extensionless scripts. What else could we do?
> > 
> > Can you - or anyone esle - think of a easy-to-use API to find the mime
> > stuff in the background? KMimeType is supposed to work from a background
> > thread after all (I asked David Faure a while ago). That would probably
> > require changes in the document controller, such that it calls
> > parseDocumentsInternal in the background somehow. Of course, this would
> > then mean we need to load the lang plugins somehow beforehand, in
> > contrast to the lazy-loading we do nowadays (afair?).
> > 
> > What do you think?
> 
> What I ment was as simple as: Adding a "QHash<IndexedString, QString>
> urlToMimetype;" in LanguageController, and calling
> "languageController->setUrlMimetype(url, mime);" from within the
> DocumentController when the document is opened, and
> languageController->clearUrlMimetype(url);" when the document is
> closed. Then it would at least work perfectly for all files that are
> open in the editor.

This looks really ugly API-wise to me. I'd prefer listening to the 
documentLoaded/Unloaded signals in the lang controller instead. Due to 
sig/slot stuff it should all happen in the main thread then and would 
encapsulate it all inside the lang controller without messing up the public 
API.

> Actually, reading the mimetype from the background is not the problem,
> the main problem is that the background-parser does it from within the
> foreground, which _kills_ the UI when there are many unparseable files
> in the project on cold disk-cache. The problem is that the mime-type
> is required _before_ starting the parse-job, because it determines
> which language-support will be used. A solution would be moving the
> whole background-parser loop into an own thread, which might bring new
> synchronization problems though.

Yes, that is exactly what I meant. Anyways, I don't see another solution, as 
the current situation is simply not OK. Sigh, I'll try to get it running :-/

bye
-- 
Milian Wolff
mail at milianw.de
http://milianw.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20110611/06d34393/attachment.sig>