[cmake-developers] Performance problem in tll-set-include-and-defines-requirements

[Stephen Kelly]

Hi, 

while building kdelibs (4), I noticed a performance problem introduced by 
my 
topic. Configuring now takes an unacceptable amount of time.

I found that this fixes it (but obviously scuttles the feature):


diff --git a/Source/cmTargetLinkLibrariesCommand.cxx 
b/Source/cmTargetLinkLibrariesCommand.cxx
index 2ccdc01..f7d721e 100644
--- a/Source/cmTargetLinkLibrariesCommand.cxx
+++ b/Source/cmTargetLinkLibrariesCommand.cxx
@@ -312,8 +312,8 @@ cmTargetLinkLibrariesCommand::HandleLibrary(const char* 
lib,
                                         ? "$<EXPORT_NAMESPACE>" : "") + 
lib;
   if (tgt || isGenex)
     {
-    this->Target->AppendTLLIncludeDirectories(compileProperty(nsLib,
-                                      "INCLUDE_DIRECTORIES", llt));
+//     this->Target->AppendTLLIncludeDirectories(compileProperty(nsLib,
+//                                       "INCLUDE_DIRECTORIES", llt));
     this->Target->AppendProperty("COMPILE_DEFINITIONS",
                       compileProperty(nsLib,
                                       "COMPILE_DEFINITIONS", llt).c_str());


Digging deeper, I found that the slowness comes from a call to 
ExpandVariablesInString(dirs, true, true); in cmMakefile::ExpandVariables() 
when called to process the INCLUDE_DIRECTORIES property of targets. 
Commenting that out restored acceptable performance.

I haven't dug any deeper into why calling that method is so slow. I assume 
it is hitting slow code paths in the parser because of the '$' which are 
not part of variables, but are generator expressions.

The patch that added that line is d899eb71b52616c9e3f3f5987f229619605e632b. 
It is apparently there for strict backwards compatibility, but it is not 
clear what exactly needs to be backwards compatible there. Any idea about 
that? 

My experiments (setting the INCLUDE_DIRECTORIES directory property and the 
INCLUDE_DIRECTORIES target property) have shown that variables are 
already expanded at that point always, so I'm guessing a the 'strict 
backwards compatibility' referred to in the commit messgae relates to a 
policy? 

If we can find out what backward compatibility that code is there to 
maintain, we should wrap it in a conditional and avoid calling it.

Thanks,

Steve.