[kdev-clang] tests: Adapt unit test to the recent changes regarding environments.
Milian Wolff
mail at milianw.de
Wed Aug 27 13:53:56 UTC 2014
On Wednesday 27 August 2014 15:04:13 Milian Wolff wrote:
> On Wednesday 27 August 2014 16:27:38 Sergey Kalinichev wrote:
> > On 8/27/14, Milian Wolff <mail at milianw.de> wrote:
> > > On Wednesday 27 August 2014 11:29:21 Sergey Kalinichev wrote:
> > >> Git commit 575013c26d3684b5af125cb9007b50bb81e2edde by Sergey
> > >> Kalinichev.
> > >> Committed on 26/08/2014 at 17:54.
> > >> Pushed by skalinichev into branch 'master'.
> > >>
> > >> Adapt unit test to the recent changes regarding environments.
> > >
> > > Can you explain this change please? I don't understand it.
> > >
> > > Also, thinking about your "memory optimization patch" a bit more, is the
> > > hash
> > > still calculated properly? I.e. are you sure that a file won't get
> > > reparsed
> > > on
> > > startup, when the same set of include paths are given but only a partial
> > > subset of these was used? I have the feeling that this might be broken
> > > now.
> >
> > Yes, I won't get updated as now I first calculate used include paths
> > and only then add it into the environment.
>
> Are you sure? I'll have to test this a bit more later, but I can't see how
> it could possibly work right now:
>
> include paths: A B C
> file gets parsed, only uses A
> -> only A in environment set
> -> environment gets serialized and stores a "hash" of A only
>
> restart kdevelop:
> include paths: A B C
> parse job compares the hash of that set to the serialized version, it
> differs -> file gets reparsed which is _wrong_
OK, I just looked at it and you indeed broke it - the failing unit test which
you "adapted" was the sign for that! Instead, you changed the unit test which
was a very bad decision of you.
Please, pretty please - don't ever do something like that ever again in the
future. This should have set off all alarm bells. You should have stopped
right there and _at least_ sent a mail here to ask what the correct behavior
would be. I wrote this test explicitly to check this functionality after all.
I will revert the memory improvement patch for now as it breaks the
functionality. Please rethink how we can improve the situation. Did you create
a massif/heaptrack log to look at where the memory is actually spent? If, as
you say, it's in some QString related stuff then we can/should make use of
Qt's implicit sharing functionality to reduce the memory consumption.
Bye
--
Milian Wolff
mail at milianw.de
http://milianw.de
More information about the KDevelop-devel
mailing list