ForegroundLock

Sun Jan 9 22:15:05 UTC 2011

Why should the foreground lock be responsible for any UI blocking?
Technically, it allows us the _minimal_ amount of UI blocking possible. I
don't understand where you want to save anything with another technique.

What you write regarding responsiveness doesn't make sense. Just to repeat
it: _everything_ that is done with the foregroundlock held, _has to_ be done
in the foreground! Kate API is not thread-safe! So there is simply nothing
you can gain by using another pattern. You have to do those calls in the
foreground anyway. The only thing one could do is making more API
thread-safe. Then the foreground lock wouldn't be required for that API,
still no need for another "pattern".

Anyway, this lock is mostly used for ligh-weight calls like
editor->getText(), and I am very sure that it doesn't have _anything_ to do
with nonresponsive UI. The duchain locks are much more problematic regarding
this.

How Qt-Creator works is not interesting, it's an entirely different
architecture, and that it's faster has nothing to do with this "pattern",
but rather with the verbosity of its stored data, and with the level of
processing. The fact that the kdev4 language architecture seems _still_
lightyears ahead of qt-creator makes this comparison even less interesting.

Regarding debugging: The foreground lock is very simple. The log I read
shows that thiago/till _nearly_ understood its internals after just an hour
or something. Apart from that, it doesn't seem to have bugs anyway. ;-)

So before you invest work into changes, please at least explain how you want
to gain something. Until now this all seems like a witch hunt, with the only
argument "I've heard the best way of doing it is X. I don't know the
difference, but it's different, so it must be better."

Greetings, David

Am 09.01.2011 22:24 schrieb "Milian Wolff" <mail at milianw.de>:

David Nolden, 07.01.2011:

> Hi, I've read your IRC discussion about the foreground lock on:
> http://webcache.googleuserconten...
First up, thanks for this email, really. I appreciate it greatly. I just
wanted to add some comments.

> - What's it's sense?
<snip>

> The second solution would be doing those actions _before_ forking into
> the background by collec...
Imo the first paragraph is not really noteworthy, but the last one explains
why the ForegroundLock is neat. It made the port much easier (and might be
used in different places in future). I still hope for the day where we
rewrite
the parse jobs though to use the other pattern you describe. And I think it
is
possible and would probably lead to a much more responsible user interface.
KDevelop *is* noticably slower when writing, and the ForegroundLock (or the
SmartMutex) are imo the #1 culprits for this. If we can dump parsejobs into
the background and let them do their job completely separated from the rest
(beside the frequent DUChain locks), I bet it will be noticably more
pleasent.

Anyhow, besides this purely theoritical performance gain, there is another
point: Maintainability. I am still, even after the lengthy discussions and
hours spent on it, not confident to fix bugs in the lock - if there are any
left. Otoh, the memory pattern is easy and straight forward to debug. That
alone is for me personally a huge advantage.

> The ForegroundLock is basically simply a generalization of the third
> solution that requires nea...
I'd like to at least assert on the duchain lock. Other locks are not global
enough to be tested there.

> - Why the ugly nested class?
> Sometimes the foreground starts nested event loops. Those are ugly...
Is it really not doing *anything*? Some events will still bubble though, or?
Also, while there is already a short comment in the code, could you maybe
add
this more elaborated explanation/justification?

> - Why the custom recursion counter?
> Only this way we can unlock the foreground lock completely ...
I think Atomic is required. If N threads do the check above and one of them
changes holderThread, it might result in undefined behavior, or not? Isn't
this exactly the kind of thing AtomicInt is supposed to be used for?
Anyways,
I'm sure Helgrind warned about this read before (without the AtomicInt).

> - Isn't it slow? We're blocking the foreground!
> Only simple and fast operations should be perfo...
Right, but it's left to be seen whether a *good* approach to the memory
pattern wouldn#t be better still. I continue to think so until the glory day
someone implements it ;-)

> - Issues
> About the _only_ issue that I know of regarding the ForegroundLock is
> that any QObje...
I didn't mean to rewrite the ForegroundLock, I meant the whole parsing API
and
I'm perfectly aware that this is borderline madness. I won't tackle it
anytime
soon, so no worries ;-) But as you can see above I still assume the memory
pattern would be the better approach. It works *very* well in QtCreator, I
cannot see how it shouldn't work for us. But this is the usual "I'd think
so"
kind of argument. I have never done anything alike before so I might be
totally wrong, who knows. I just got lots of feedback by a number of people
I
respect for their programming knowledge, all of them urging me to use the
memory pattern. I doubt they are all wrong. And just to make sure this is
not
misunderstood: I love how you implemented this and ported KDevelop. I bet we
would still be changing API instead of concentrating on bug fixes and
releasing KDevelop 4.2 soon (which reminds me, I should start creating the
tarballs for RC1).

So let me finish it with a big thank you again. You did tremendously good
work, and I do hope you continue to come up with such interesting and
effectic
solutions to our problems.

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

--
KDevelop-devel mailing list
KDevelop-devel at kdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20110109/53399058/attachment.html>