Review Request: Patch to reduce C++ parser memory consumption from 3% to 35%

David Nolden zwabel+reviewboard at gmail.com
Tue Dec 22 23:15:10 UTC 2009


-----------------------------------------------------------
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
> 
>





More information about the KDevelop-devel mailing list