Review Request: Patch to reduce C++ parser memory consumption by removing parent AST pointer from AST nodes

[Alexander Dymo]

> On 2010-02-15 15:07:39, David Nolden wrote:
> > I'm against removing this, because it is a functionality that is very useful. Yes we don't use it right now, but saving 8% of temporary-memory for one extreme test-case is IMO not enough motivation to completely remove this functionality.
> > That said, an alternative structure like a hash-map might be the better approach in long-term.

So, you agree that
 1) we don't use it now and will not use it in 4.0 
 2) hash-map is the way to go

So, let's just remove the pointer. It adds no value right now. One can easily implement the hash approach. I can even do that if you want (in the way I did in my other patch).


- Alexander


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


On 2010-02-15 14:04:31, Alexander Dymo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2940/
> -----------------------------------------------------------
> 
> (Updated 2010-02-15 14:04:31)
> 
> 
> Review request for KDevelop, Bertjan Broeksema and David Nolden.
> 
> 
> Summary
> -------
> 
> We currently have AST *parent pointer in every AST node and it's never used in our code (I mean the release-ready code). Bertjan, I know that's something you wanted for your work, but maybe you can implement another way of storing parents (a map perhaps?). This unused pointer increases the size of AST and I'd like to avoid things like that as much as possible.
> 
> For my large C file, the peak memory consumption is reduced from 703M to 655M (-48M) by removing the parent pointer.
> 
> 
> Diffs
> -----
> 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/cppparsejob.cpp 1090158 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/parser/CMakeLists.txt 1090158 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/parser/parentvisitor.h 1090158 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/parser/parentvisitor.cpp 1090158 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/parser/parsesession.h 1090158 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/parser/parsesession.cpp 1090158 
> 
> Diff: http://reviewboard.kde.org/r/2940/diff
> 
> 
> Testing
> -------
> 
> All tests pass.
> 
> 
> Thanks,
> 
> Alexander
> 
>