[kdev-clang] duchain: Reduce memory consumption.

[Milian Wolff]

On Wednesday 27 August 2014 11:29:19 Sergey Kalinichev wrote:
> Git commit 66882ceb15fe8cc4271fa3a50144c451aa0908d8 by Sergey Kalinichev.
> Committed on 24/08/2014 at 16:35.
> Pushed by skalinichev into branch 'master'.
> 
> Reduce memory consumption.
> 
> Before this commit we stored absolutely identical contexts for system
> files (e.g. QString) with different include directories in the
> environments which not used by the given file at all (e.g. some
> project specific include directory)
> 
> Now we add into environment only those include directories, that's
> really used by the given file.
> 
> Here the rough test results for a middle size project:
> 
>         No files      1 file opened        After restarting
> Before:  378             540                     250
> After:   252             341                     176

Nice, that is a very good idea Sergey - thanks for the patch! I do have some 
comments though (see below). Please put patches up for review first.

> M  +54   -1    duchain/clanghelpers.cpp
> 
> http://commits.kde.org/kdev-clang/66882ceb15fe8cc4271fa3a50144c451aa0908d8
> 
> diff --git a/duchain/clanghelpers.cpp b/duchain/clanghelpers.cpp
> index f6027da..96ad4d1 100644
> --- a/duchain/clanghelpers.cpp
> +++ b/duchain/clanghelpers.cpp
> @@ -78,6 +78,37 @@ CXChildVisitResult visitCursor(CXCursor cursor, CXCursor,
> CXClientData data) return CXChildVisit_Recurse;
>  }
> 
> +struct ImportsForFile {

Style: Put open-brace on newline

> +    CXFile file;
> +    QVector<CXFile> imports;
> +};
> +
> +CXChildVisitResult visitImportsInFile(CXCursor cursor, CXCursor,
> CXClientData data)
> +{
> +    if (cursor.kind != CXCursor_InclusionDirective) {
> +        return CXChildVisit_Continue;
> +    }
> +
> +    auto imports = static_cast<ImportsForFile*>(data);
> +    {

A comment would be helpful: "skip includes in other files".

> +        auto location = clang_getCursorLocation(cursor);
> +        CXFile file;
> +        clang_getFileLocation(location, &file, nullptr, nullptr, nullptr);
> +        if (file != imports->file) {
> +            return CXChildVisit_Continue;
> +        }
> +    }
> +
> +    CXFile file = clang_getIncludedFile(cursor);
> +    if(!file){
> +        return CXChildVisit_Break;


Why do you break? Shouldn't you continue?

> +    }
> +
> +    imports->imports.append(file);
> +
> +    return CXChildVisit_Recurse;
> +}
> +
>  ReferencedTopDUContext createTopContext(const IndexedString& path, const
> ClangParsingEnvironment& environment) {
>      ClangParsingEnvironmentFile* file = new
> ClangParsingEnvironmentFile(path, environment); @@ -117,7 +148,29 @@
> ReferencedTopDUContext ClangHelpers::buildDUChain(CXFile file, const
> Imports& im
> 
>      const IndexedString
> path(QDir::cleanPath(QString::fromUtf8(ClangString(clang_getFileName(file))
> )));
> 
> -    const auto& environment = session.data()->environment();
> +    auto environment = session.data()->environment();
> +
> +    ImportsForFile importsForFile;
> +    importsForFile.file = file;
> +
> +    if
> (!clang_visitChildren(clang_getTranslationUnitCursor(session.unit()),
> &visitImportsInFile, &importsForFile)) { +        // Add only used include
> directories into the environment. +        KDevelop::Path::List includes;
> +        for (const auto& includePath: environment.includes()) {
> +            for (const auto& import: importsForFile.imports) {
> +                if
> (QDir::cleanPath(QString::fromUtf8(ClangString(clang_getFileName(import))))
> .startsWith(includePath.toLocalFile())) { +                   
> includes.append(includePath);
> +                    break;
> +                }
> +            }
> +        }
> +        ClangParsingEnvironment env;
> +        env.addDefines(environment.defines());
> +        env.addIncludes(includes);
> +        env.setProjectKnown(environment.projectKnown());
> +        //kDebug() << "Reduced includes from: " << environment.includes()
> <<  "\nto: " << env.includes() << "\nfor: " << path.str() << " project: "
> << env.projectKnown();

remove this debug output line or comment it out, but don't leave commented 
code around

> +        environment = env;

Generally, I'd put this block into a free function environmentForFile or 
similar, which then returns either the environment you create here or the 
session environment as-is as a fallback. That way we can do:

const auto& environment = environmentForFile(file, session);

Bye
-- 
Milian Wolff
mail at milianw.de
http://milianw.de