gtg-contributors team mailing list archive

[Lionel Dricot <ploum@xxxxxxxxx>]

Hello,

First I want to point out that I merged liblarch_newbase in to
liblarch_rebased a while ago. (and I removed liblarch_rebased at that time
to be sure)

Liblarch_newbase is then out of date. Would it be easy for you to merge
all your commits in liblarch_rebased ?  If not, we might consider a merge.



On Tue, 7 Sep 2010 16:56:08 +0200, Luca Invernizzi
<invernizzi.l@xxxxxxxxx>
wrote:
> Hello Lionel (and CCd gtg contributors),
> I'm trying to help in the development of liblarch_newbase, but I'd
> like to hear from your experience: what do you think is not working
> properly (also, in terms of performances)? Don't be afraid of being
> lenghty, I'll read it all :)

First of all : thanks Luca. I mean: thanks, thanks, thanks.



> Also, I've made one one-liner commit I'd like you to comment on. I've
> noticed that deleting a task/node sitting on top of a very long list
> was causing huge delays (depending on the number of nodes in the
> list), while deleting from the bottom was fine. That's because
> liblarch is updating all the siblings nodes that follow the one that
> is deleted. Removing that behaviour, however, doesn't seem to break
> anything: the tests are fine and the gtk treeview is too. Am I wrong?

Check it in liblarch_rebased (the real one)  because I've fixed that kind
of issue and improved performance dramatically in that branch.

If it is not fixed in liblarch_rebased : good news. It means yet another
improvement :-)  (check : no, it wasn't fixed. Good catch !)


The reason for that is simple : when I started to dig deeply into
TreeModel, I realized that, when you delete a node X, all siblings have
their path changed. So I needed to remove them first then to re-add them.
It is ugly but I wanted to be sure to have something working fine before
optimizing.

Later, I discovered that there is a "child reordered" method in
gtk.TreeModel. So, in order to be more efficient, before deleting a node, I
reorder them to put the node to delete at the end of all siblings.  (You
might say that gtk.TreeModel should do that automatically. Indeed, it
should. But so far, I've no proof that it does effectively, that's why I
take a very careful approach)


I believe that the function you commented out was left from the time I was
manually removing/reinserting siblings to delete a node.

Anyway, in my new philosophy : if it doesn't break a test, it is valid.
Before your failing test, I had still a lot of bugs but all tests were
failing. The goal is then to write failing tests.


> I've also added a failing test I'd like you to see. It's a stress
> test, adding lots of nodes, updating them and then deleting them. The
> "delete" part can't find the nodes in the tree, but the tree says
> they're present.

I will have a look at that as soon as you've merged your change in
liblarch_rebased. That's very very very interesting. We should aim for
test, test, test and only test.


> Thirdly, I've seen that performances are bad when adding a lot of
> tasks (with ./scripts/stress_test  1000), but loading them from disk
> is very fast. This is kind of strange. Anyway, this bug won't be
> visible to GTG users (I don't think they type fast enough), so it's
> lower priority.

There's one big problem currently : liblarch is not thread-safe (and could
not be, IMHO). I've changed every thread call by a direct call (see last
changes in liblarch_rebased) so it works well but slowly. The problem is
that every liblarch call is now done with a gobject_idle_add, thus
effectively computed in Gtk.MainLoop which also renders the widget.

Ideally, liblarch would be in its own thread (which should be the GTG
main's thread) and should be different of the UI thread. I believe it would
improve stuffs dramatically.

PS: all that threads stuffs are controlled by constant at the start of
each file. It's then easy to test and change stuffs. 


> Lastly, do you think we're ready to add multi-selection and drag-n-drop?

I think it can be started. Honnestly, I don't fear it. It would be a
simple copy/paste/adaptation of the current drag-n-drop code into
liblarch_gtk.TreeView. Basically, it will be nothing else than function
that move a node or add a parent (I think that it should be configurable :
0=no drag-n-drop, 1=move node on drag-n-drop 2=copy node on drag-n-drop).



My real concern is that liblarch_rebased was passing all the tests but was
still very crashy. There must be something I still miss there.


Don't hesitate if you have any other question.