QML/JS aliases vs. imports

[Milian Wolff]

On Friday 11 July 2014 16:11:57 Denis Steckelmacher wrote:
> Hi,
> 
> I've removed all the alias declarations that were used by the QML/JS
> plugin, and I replaced them with imported parent contexts (where
> applicable) and the use of the persistent symbol table.
> 
> I think that Reviewboard does not allow review requests for commits
> living in scratch repositories, so here is a commit on which I would
> like some advice :
> http://quickgit.kde.org/?p=scratch%2Fdsteckelmacher%2Fkdev-qmljs.git&a=commi
> tdiff&h=c5e1efd0c9120c27a8e5ede5879e12b4da307bb4 .

- don't break the first comment, below are lines that are longer, this way it 
just looks odd
- initialize variables, always. uint count = 0; declarations = nullptr;

Also, you must hold the duchain read lock while operating on the cache, at 
least the check for isValid on the indexed context is otherwise meaningless. 
I'd just pull out the read lock and put it above the symbol table lookup call

> This commit uses PersistentSymbolTable to find the declaration of global
> identifiers. Everything works well, but I'm not very happy with how the
> "two files are in the same directory" condition works. I'm obliged to
> call IndexedString::str, then QDir::fromNativeSeparator and finally
> QString::section in order to finally get the directory path of the file.
> Is there a quicker way of doing this?

why do you even filter based on the directory? if at all, prefer stuff that is 
located near the current file, but don't exclude anything. You can never be 
sure what can be imported where without executing stuff. In QML e.g. you can 
define custom import paths etc. pp.

that said, you can also compare URL's, or probably better, Paths:

const Path dir = Path(currentDocument.toUrl()).parent();
...
if (!dir.isDirectParentOf(Path(context->url().toUrl()))) {
  continue;
}

> Regarding the speed of my solution, the testsuite of the plugin (that
> parses tens of different files, plus several QML modules) ran in 5.36
> seconds on my computer before my commit. With the commit applied, the
> testsuite runs in 5.40 seconds, so the impact of the commit seems to be
> negligible, and PersistentSymbolTable seems to be very fast. Can someone
> confirm that what I've done is correct?

Without a proper benchmark, this test is meaningless. You'd have to:

- build everything (esp. kdevplatform) in release mode
- run duchainify on a big project that uses a lot of QML/JS
- compare before/after, while averaging over a few runs each and taking the 
variance into account.

That said, I don't think its going to be that slow. The PersistentSymbolTable 
is written for speed.

Bye
-- 
Milian Wolff
mail at milianw.de
http://milianw.de