[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kdevelop-devel
Subject:    Re: Review Request: Patch to reduce C++ parser memory consumption
From:       "David Nolden" <zwabel+reviewboard () gmail ! com>
Date:       2009-12-22 23:15:10
Message-ID: 20091222231510.15887.10404 () localhost
[Download RAW message or body]


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2436/#review3499
-----------------------------------------------------------

Ship it!


Generally it looks ok. The only problem I see with letting the "ast" zero at the \
beginning is that we have to make absolutely sure the "UPDATE_POS(..)" stuff will \
never be reached as long as it's zero.

- David


On 2009-12-21 23:27:42, Alexander Dymo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2436/
> -----------------------------------------------------------
> 
> (Updated 2009-12-21 23:27:42)
> 
> 
> Review request for KDevelop.
> 
> 
> Summary
> -------
> 
> KDevelop C++ parser may consume a lot of memory because it sometimes creates AST \
> nodes on the pool which aren't used later. For example, in some cases it tries to \
> parse simple type specifier, creates a node for it, then decides that the stuff \
> under the cursor isn't a type specifier and returns from the method leaving AST \
> node in the pool. Other methods (parseName, parsePrimaryExpression, etc.) have the \
> same mistake. 
> Usually that's not the problem, but I have a 4M file which consists solely from \
> array variable definitions with initializations. There're ~1 million integers for \
> those arrays in the file and parser tries to figure out whether each of those \
> integers is a type specifier. Of course they are not and the parser left ~100M of \
> unused SimpleTypeSpecifierAST nodes. Same happened in other methods. 
> This patch fixes the problem, but it needs review (David?, Hamish?). I just don't \
> know C++ parser enough to be sure I didn't break anything. 
> Also, this patch doesn't fix all such problems. If my change is correct, I'll \
> continue with fixing the remaining places. 
> 
> Diffs
> -----
> 
> /trunk/extragear/sdk/kdevelop/languages/cpp/parser/parser.cpp 1064830 
> 
> Diff: http://reviewboard.kde.org/r/2436/diff
> 
> 
> Testing
> -------
> 
> On my installation, 4 duchain tests were broken before this change. After this \
> change I got no new broken tests. 
> Performance and memory consumption investigation:
> 
> == parsing my test file ==
> before:	552M	3.84 sec
> after:	364M	3.59 sec
> 
> == parsing the whole kdevplatform ==
> before:	299M	92.93 sec
> after:	291M	92.75 sec
> 
> Result: memory consumption decreased by 35% in my case and 3% in case of \
> kdevplatform. As a side effect, the parser became a bit faster. 
> 
> Thanks,
> 
> Alexander
> 
> 


-- 
KDevelop-devel mailing list
KDevelop-devel@kdevelop.org
https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel


[prev in list] [next in list] [prev in thread] [next in thread]