[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