Using DUChain from the main thread

[Hamish Rodda]

On Tue, 30 Oct 2007 06:57:38 am David Nolden wrote:
> On Monday 29 October 2007 14:10:15 Hamish Rodda wrote:
> > **** Background:
> > Currently, the duchain is accessed from the main thread by the DUChain
> > users (eg, the duchainviewer).  They receive notifications directly from
> > the parsing thread about changes, and thus has to protect themselves
> > during a notification, as eg. it could also be within a request from the
> > item model system.  This makes it pretty hard to comply with the item
> > model's requirements for item addition, etc.  So, we end up having to
> > create a separate tree for each node in the chain, and make sure it is
> > not deleted each time it is accessed.  Any lengthy code execution here
> > will block the parsing thread.  To properly comply with the item model's
> > requirements, a complex system would be required.
>
> I probably don't know enough about the model/view thread-safety to exactly
> know what the problems you are talking about are.
> But blocking the parsing-thread for a short time should not really hurt,
> because the user shouldn't really notice it.

Speaking of blocking the main thread, is the code completion stuff still done 
in the main thread?  If so, I'm tempted to move it out...

> > **** Idea:
> > As we want to remove as much of the tricky code from the DUChain users
> > (eg class browser) as possible, make sure that the notifications are
> > delivered within the thread that the DUChain user resides in.  This is
> > achieved by the following:
> >
> > 1) each time a change is made, the duchain emits a signal from the
> > parser's thread
> > 2) this is connected to by users, and received in their thread, not the
> > parser's thread (Qt::QueuedConnection)
> > 3) a separate, private signal is emitted after a duchain object is about
> > to be deleted, and then the object is deleted in the main thread.
> >
> > The guarantees that are provided to the DUChain user:
> > 1) objects will not be deleted prior to you receiving a signal that warns
> > you they are about to be deleted
> > 2) any other detail about them may change, unless you hold a read-lock
> > over the duchain.
>
> Hmm so the only guarantee is that the adress of an object stays same, while
> any other detail may change.
>
> Did you already notice the DUChainPointer class? It is a self-invalidating
> smart-pointer that works for all duchain items, and does not need a
> notification system. After an object is deleted, all corresponding
> DUChainPointers will have an effective value of zero, but it still will be
> possible to find out whether they have pointed to the same target when it
> still existed(They use an intermediate pointer, that could also be used in
> maps, sets, etc.). Wouldn't this be perfectly fine for such an asynchronous
> notification system? After an item was deleted, the signal would deliver a
> zero DUChainPointer, but that could still be used to locate the part of the
> tree that needs to be removed. While this sounds kind of useless, it would
> effectively be the same guarantee. :)

I thought about it more, and you're right, this is a good way to go.  It's 
easier to write bug-free code compared to playing with bare pointers.  My 
main concern was having to duplicate the displayed parts of the duchain 
whenever you want to provide it as a model, but I've realised that this has 
to be done anyway.

(BTW, why does the DUChainBase object set m_base, even though it already 
passes the pointer to itself in the constructor?)

> > This still requires a separate tree of objects, but hopefully will be
> > less complicated and easier to maintain.
> >
> > The main drawback that I can see with this technique is that deletion of
> > objects will be slower, as two signals have to be emitted for each object
> > deletion.  However, I would think this will be an uncommon use, and when
> > mass deletions are required, we could turn the notification system off
> > and send a reset() signal.
> >
> > Well, I hope that's clear... anyone with ideas / comments please speak
> > up...
>
> I'm a bit worried about general performance of the whole duchain
> signal/slot notification stuff with event-queues, because duchains can be
> quite long, and the event-queues might fill up for example when unloading a
> whole
> duchain(which currently we don't do, but we will need to do it some time
> ;), or when parsing a lot of dependencies.
>
> I don't like the idea that the class-view performs complex actions after
> each parsed function, it would be better if it could batch the whole
> process, and do big changes at one time.
>
> Using DUChainPointer, you could do the whole actual view management in this
> way, while keeping the signals synchronous:
> - Maintain a set of changed DUChain items using DUChainPointer. Whenever a
> signal is received from the duchain, be it an added/deleted/changed
> notification, add the corresponding item to that set
> - After a specified time, synchronize the tree-view by using the stored set
> of changes. Since we are using DUChainPointer, we will notice deleted items
> by their zero values, without crashing. If they are not zero, update/add
> the data.

At the moment I'm implementing an as-you-go system, we can profile it later if 
it's a problem and it will be trivial to make it work in batches.  Probably 
best to make it work both ways in the end, because you want responsiveness 
when writing code (if we ever get incremental parsing working :).

> An important optimization here would be to, while parsing new files, only
> put the TopDUContext for the file into that set, not each of its sub-nodes.
>
> Btw. what are your current ideas of the class-view? Anything
> revolutionary? :-)

Mostly to get it to work first, then to use it as a gateway to Cool Stuff(tm) 
like refactoring.

Cheers,
Hamish
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20071031/5ad87e12/attachment.sig>