Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-08-06 Thread Nicolas Desprès
Hi Brad,

I have force-pushed all the fixes you've asked for:
* I taught cmMakefile copy-ctor
* I re-write the test so that it is part of CMakeOnly sub-test suite. I
also make it more robust to machine load by increasing the number of
targets. This way the performance gap between the logarithmic version and
the linear version is bigger. On my machine the logarithmic version run in
60sec whereas the linear version had not finished after 10min. The timeout
is at 90sec now. When running the whole test suite in parallel with 8 jobs
it passes.
* I added a test covering the issue I got while testing the previous buggy
version of the optimized algorithm. This is a very weird bug. Master does
not suffer from it, neither this topic branch. Basically the previous
version had problem with empty dependency and it was not covered by the
test suite. I think there is something else involved in this test case
because the test file named 0 is important too. At least the test is here
now and we could investigate that later if it fails again.

Let me know what do you think.

Thanks,
-Nico



On Mon, Aug 5, 2013 at 10:50 PM, Brad King brad.k...@kitware.com wrote:

 On 08/05/2013 04:37 PM, Nicolas Desprès wrote:
  * Why is special logic in Tests/CMakeLists.txt needed to add
the test case?
 
  Since it is a performance test on configuration phase only, I need a
  specific timeout value and to run cmake only

 Take a look at the Tests/RunCMake and Tests/CMakeOnly groups of
 tests.  They are for running CMake without building.

 -Brad




-- 
Nicolas Desprès
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-08-06 Thread Brad King
On 08/06/2013 09:40 AM, Nicolas Desprès wrote:
 I have force-pushed all the fixes you've asked for:

Great, thanks!

 * I taught cmMakefile copy-ctor

Good.

 * I re-write the test so that it is part of CMakeOnly sub-test suite. I also 
 make it more robust to machine load by increasing the number of targets. This 
 way the performance gap between the logarithmic version and the linear 
 version is bigger. On my machine the logarithmic version run in 60sec
 whereas the linear version had not finished after 10min. The timeout is at 
 90sec now. When running the whole test suite in parallel with 8 jobs it 
 passes.

Okay.  Timing-dependent tests have been frequently sporadic on
our test infrastructure because the machines are often busy
with other projects' tests and there is a wide variety of
machine speeds available.  We'll see how it goes.

 * I added a test covering the issue I got while testing the previous buggy 
 version of the optimized algorithm. This is a very weird bug. Master does not 
 suffer from it, neither this topic branch. Basically the previous version had 
 problem with empty dependency and it was not covered by the test
 suite. I think there is something else involved in this test case because the 
 test file named 0 is important too. At least the test is here now and we 
 could investigate that later if it fails again.

It looks like

 +  add_test_macro(EmptyDepAndZeroOutput
 +${CMAKE_CMAKE_COMMAND} -P check.cmake)

appears twice, once in each test addition commit.

While hunting down all the call sites for GetSourceFileWithOutput
I found two that are not needed and removed them:

 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=eccb39d7

Please rebase your topic on that.

I still can't convince myself that the comparison function is
correct.  Also as Steve suggested it will be faster to use a
standard ordering function.  That should work if we always
use it with full paths.  The expected use cases always use
full paths and it is only for backward compatibility that we
support the path suffix magic.  Therefore I think the best
approach is to optimize the common (full-path) case with a
direct lookup, perhaps even with cmsys/hash_map.hxx.  Only
when the input path is relative should we fall back to the
linear suffix search for compatibility.

Thanks,
-Brad
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-08-06 Thread Nicolas Desprès
On Tue, Aug 6, 2013 at 4:31 PM, Brad King brad.k...@kitware.com wrote:

 On 08/06/2013 09:40 AM, Nicolas Desprès wrote:
  I have force-pushed all the fixes you've asked for:

 Great, thanks!

  * I taught cmMakefile copy-ctor

 Good.

  * I re-write the test so that it is part of CMakeOnly sub-test suite. I
 also make it more robust to machine load by increasing the number of
 targets. This way the performance gap between the logarithmic version and
 the linear version is bigger. On my machine the logarithmic version run in
 60sec
  whereas the linear version had not finished after 10min. The timeout is
 at 90sec now. When running the whole test suite in parallel with 8 jobs it
 passes.

 Okay.  Timing-dependent tests have been frequently sporadic on
 our test infrastructure because the machines are often busy
 with other projects' tests and there is a wide variety of
 machine speeds available.  We'll see how it goes.

Sure.


  * I added a test covering the issue I got while testing the previous
 buggy version of the optimized algorithm. This is a very weird bug. Master
 does not suffer from it, neither this topic branch. Basically the previous
 version had problem with empty dependency and it was not covered by the test
  suite. I think there is something else involved in this test case
 because the test file named 0 is important too. At least the test is here
 now and we could investigate that later if it fails again.

 It looks like

  +  add_test_macro(EmptyDepAndZeroOutput
  +${CMAKE_CMAKE_COMMAND} -P check.cmake)

 appears twice, once in each test addition commit.

Oups. Squashing error. Fixed now.


 While hunting down all the call sites for GetSourceFileWithOutput
 I found two that are not needed and removed them:

  http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=eccb39d7

 Please rebase your topic on that.

 I still can't convince myself that the comparison function is
 correct.  Also as Steve suggested it will be faster to use a
 standard ordering function.  That should work if we always
 use it with full paths.  The expected use cases always use
 full paths and it is only for backward compatibility that we
 support the path suffix magic.  Therefore I think the best
 approach is to optimize the common (full-path) case with a
 direct lookup, perhaps even with cmsys/hash_map.hxx.  Only
 when the input path is relative should we fall back to the
 linear suffix search for compatibility.

Good idea! I should have done that in the first place it would have save me
a lot of effort understanding the old behavior.

 The test suite passes on my computer. I have force-pushed again.

However I had one problem with BootstrapTest when trying to use
cmsys::hash_map. I pushed by work so far on this topic in another branch (
https://github.com/nicolasdespres/CMake/commits/topic/use-cmsys-hash_map):
https://github.com/nicolasdespres/CMake/commit/1437d51a87b5a5a131b3829210710734266d29cc

I did not succeed to teach the bootstrap script how to include cstddef.

Cheers,

-- 
Nicolas Desprès
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-08-06 Thread Brad King
On 08/06/2013 12:18 PM, Nicolas Desprès wrote:
 The test suite passes on my computer. I have force-pushed again.

Great.

In this hunk:

+  // If the queried path is not absolute we use the backward compatible
+  // version. The search algorithm is linear.
+  if (cname[0] != '/')

Use cmSystemTools::FileIsFullPath to detect a full path in a way that
works on Windows too.

 However I had one problem with BootstrapTest when trying to use 
 cmsys::hash_map

During bootstrap we can just use normal std::map.  Squash the patch
below into your topic/use-cmsys-hash_map (and then into the original
topic).

-Brad


diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h
index e51d9c6..f00f47b 100644
--- a/Source/cmMakefile.h
+++ b/Source/cmMakefile.h
@@ -29,7 +29,9 @@

 #include cmsys/auto_ptr.hxx
 #include cmsys/RegularExpression.hxx
-#include cmsys/hash_map.hxx
+#if defined(CMAKE_BUILD_WITH_CMAKE)
+# include cmsys/hash_map.hxx
+#endif

 class cmFunctionBlocker;
 class cmCommand;
@@ -1049,7 +1051,11 @@ private:
   cmSourceFile *LinearGetSourceFileWithOutput(const char *cname);

   // A map for fast output to input look up.
+#if defined(CMAKE_BUILD_WITH_CMAKE)
   typedef cmsys::hash_mapstd::string, cmSourceFile* OutputToSourceMap;
+#else
+  typedef std::mapstd::string, cmSourceFile* OutputToSourceMap;
+#endif
   OutputToSourceMap OutputToSource;

   void UpdateOutputToSourceMap(const std::vectorstd::string outputs,
diff --git a/bootstrap b/bootstrap
index 9e6bfad..afb66e5 100755
--- a/bootstrap
+++ b/bootstrap
@@ -304,8 +304,6 @@ KWSYS_CXX_SOURCES=\
 KWSYS_FILES=\
   auto_ptr.hxx \
   Directory.hxx \
-  hash_map.hxx \
-  hashtable.hxx \
   Glob.hxx \
   Process.h \
   RegularExpression.hxx \
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-08-06 Thread Brad King
On 08/06/2013 12:44 PM, Brad King wrote:
 On 08/06/2013 12:18 PM, Nicolas Desprès wrote:
 The test suite passes on my computer. I have force-pushed again.
 
 Great.

I had to modify the test like this:

 -DEPENDS bottle_neck
 +DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/bottle_neck

in order to ensure the optimization is used for the test to
finish within the timeout.

Also the test does not work well on Windows because the
filesystem is very slow to deal with a large number of
files and directories.  Even removing creation of the
input_${i} files the 1000 output-target build makefiles
take forever to write.

IMO it is not worth the effort to establish this test.
We have no performance tests elsewhere and it is unlikely
this will regress.  The way the lookup is done with your
patches is the way it should have been done originally.

-Brad
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-08-06 Thread Brad King
On 08/06/2013 01:30 PM, Brad King wrote:
 IMO it is not worth the effort to establish this test.

I've applied a patch based on yours to implement the
optimization and merged to 'next' for testing tonight:

 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=2268c41a

-Brad
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-08-06 Thread Nicolas Desprès
On Tue, Aug 6, 2013 at 10:19 PM, Brad King brad.k...@kitware.com wrote:

 On 08/06/2013 01:30 PM, Brad King wrote:
  IMO it is not worth the effort to establish this test.

 I've applied a patch based on yours to implement the
 optimization and merged to 'next' for testing tonight:

  http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=2268c41a

 Thank you very much for your help. I guess this topic is closed now. Sorry
for not answering earlier. I was away from keyboard (time shift). I will
delete my branches tomorrow.

-Nico
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-08-05 Thread Nicolas Desprès
Hi,

I have just rebased my branch on top of the master and the test suite still
pass.

I have a real world example here with performance. My project's build
system has 7113 commands to run. This is roughly 7 commands for each input
files. There are 1000 inputs files.

Just for the configuration step:
cmake latest release takes 59:50.10 minutes
cmake with my branch takes 1:06.98 minutes

So it is about an hour with the latest release and 1 minutes with the patch.

I have updated the code, so I hope it will be clearer now. I really want to
have this patch in the next release, since I cannot use cmake for my
project otherwise. I can work full time on this patch until the end of this
week if there is something to change.

Cheers,
-Nico

On Mon, Jul 29, 2013 at 2:40 PM, Nicolas Desprès
nicolas.desp...@gmail.comwrote:




 On Mon, Jul 29, 2013 at 2:25 PM, Stephen Kelly steve...@gmail.com wrote:

 Nicolas Desprès wrote:

  Also, I don't see why the custom comparison functor is needed at all. I
  removed it and sped up the test significantly. Can you explain?
 
 
  I had some failing tests because the previous algorithm was not a plain
  strcpy().

 I don't see any strcpy(). Anyway, I think that's as much useful review I
 can
 do for you. I think Brad will have to do the rest.


 Arg. Sorry for being unclear (English is not my mother tongue).

 The std::lessstring comparator do the same thing (as strcpy()  0).
 Unfortunately, we cannot use it here because the previous (O(N))
 implementation of cmMakefile::GetSourceFileWithOutput(const char*) was more
 complex than that (have a look to the code my path replaces). Of course, I
 first tried with the default comparator to save me some work but some tests
 were failing. Namely: ExportImport and CustomCommand.

 Hope this help.
 Thanks for your time.
 -Nico




-- 
Nicolas Desprès
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-08-05 Thread Brad King
On 08/05/2013 12:06 PM, Brad King wrote:
 Here are a couple comments:

Also I think the cmMakefile copy constructor needs to be taught
about the member you're adding.

-Brad
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-08-05 Thread Nicolas Desprès
On Mon, Aug 5, 2013 at 6:06 PM, Brad King brad.k...@kitware.com wrote:

 On 08/05/2013 11:28 AM, Nicolas Desprès wrote:
  I have just rebased my branch on top of the master and the test suite
 still pass.
 [snip]
  I really want to have this patch in the next release

 I can't promise to accept it since 2.8.12 rc1 is coming out
 tomorrow or Wednesday and this topic isn't even in 'next' yet.
 This touches some pretty fundamental logic and could easily
 have a subtle behavior change so I consider it high risk.

I understand. I have just found an issue that is not covered by CMake's
test suite. It happens when one of the file path is empty. I will add a
test for this case tomorrow.


 On 07/29/2013 08:25 AM, Stephen Kelly wrote:
  I think Brad will have to do the rest.

 Sorry, I stopped reading this thread assuming that it would end
 with Steve bringing the change to the topic stage at which point
 my attention would be drawn again.

No problem.


 I just fetched the large-deps-perf topic from your Github repo.
 I've never been happy with the linear search so thanks for working
 on this.  Here are a couple comments:

 * Please replace large_list.cmake in the test with use of the
   foreach() command's RANGE signature and list(APPEND).

Done.


 * Why is special logic in Tests/CMakeLists.txt needed to add
   the test case?

Since it is a performance test on configuration phase only, I need a
specific timeout value and to run cmake only (neither make, nor any extra
program built the test project). I have added a comment saying so.


 * Please add comments explaining in prose what the lookup map
   comparison functor is doing and why.  I could infer it only
   by looking at the old code (which should have been commented
   before too).  The assert examples are good for verification
   but not for understanding from scratch.

I added the comment. I hope it is understandable. The best would be to have
a unit test for this method but I can't think of a simple way to do it
since there will be too many objects to mock.

I have pushed a bunch of fixup commit if you want to review them. I will
squash them tomorrow.

Thanks,

-- 
Nicolas Desprès
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-08-05 Thread Nicolas Desprès
On Mon, Aug 5, 2013 at 9:50 PM, Brad King brad.k...@kitware.com wrote:

 On 08/05/2013 12:06 PM, Brad King wrote:
  Here are a couple comments:

 Also I think the cmMakefile copy constructor needs to be taught
 about the member you're adding.

Yep I do that tomorrow morning. It is late now in France.

-- 
Nicolas Desprès
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-08-05 Thread Brad King
On 08/05/2013 04:37 PM, Nicolas Desprès wrote:
 I have pushed a bunch of fixup commit if you want to review them. I will 
 squash them tomorrow.

Thanks!  Don't bother squashing yet.  There may be more fixups
needed after testing.

-Brad
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-08-05 Thread Brad King
On 08/05/2013 04:37 PM, Nicolas Desprès wrote:
 * Why is special logic in Tests/CMakeLists.txt needed to add
   the test case?
 
 Since it is a performance test on configuration phase only, I need a
 specific timeout value and to run cmake only

Take a look at the Tests/RunCMake and Tests/CMakeOnly groups of
tests.  They are for running CMake without building.

-Brad
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-29 Thread Stephen Kelly
Nicolas Desprès wrote:
 It was fastest because it was not doing the right thing. I tried to patch
 it properly and the benchmark are the same whether we use the default
 comparison functor or mine.
 
 So I think you can merge it like that. I have pushed a new version without
 the comment.
 

I still haven't tried it, but there are still style issues.

* Don't put an else after a return
* Wrap single line blocks in {} 

Thanks,

Steve.


--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-29 Thread Nicolas Desprès
On Mon, Jul 29, 2013 at 12:57 PM, Stephen Kelly steve...@gmail.com wrote:

 Nicolas Desprès wrote:
  It was fastest because it was not doing the right thing. I tried to patch
  it properly and the benchmark are the same whether we use the default
  comparison functor or mine.
 
  So I think you can merge it like that. I have pushed a new version
 without
  the comment.
 

 I still haven't tried it, but there are still style issues.


 * Don't put an else after a return
 * Wrap single line blocks in {}


Fixed and force-pushed. Sorry for the inconvenience. I am not used to this
style yet.

Thanks for the review.
-Nico
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-29 Thread Stephen Kelly
Nicolas Desprès wrote:

 On Mon, Jul 29, 2013 at 12:57 PM, Stephen Kelly
 steve...@gmail.com wrote:
 
 Nicolas Desprès wrote:
  It was fastest because it was not doing the right thing. I tried to
  patch it properly and the benchmark are the same whether we use the
  default comparison functor or mine.
 
  So I think you can merge it like that. I have pushed a new version
 without
  the comment.
 

 I still haven't tried it, but there are still style issues.
 
 
 * Don't put an else after a return
 * Wrap single line blocks in {}

 
 Fixed and force-pushed. Sorry for the inconvenience. I am not used to this
 style yet.

Your Compare::operator() contains this:
 
 if (j == s2.rend())
   {
   return false;
   }
 return false;

Any reason not to simplify that?

Also, I don't see why the custom comparison functor is needed at all. I 
removed it and sped up the test significantly. Can you explain?

Thanks,

Steve.


--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-29 Thread Nicolas Desprès
On Mon, Jul 29, 2013 at 2:09 PM, Stephen Kelly steve...@gmail.com wrote:

 Nicolas Desprès wrote:

  On Mon, Jul 29, 2013 at 12:57 PM, Stephen Kelly
  steve...@gmail.com wrote:
 
  Nicolas Desprès wrote:
   It was fastest because it was not doing the right thing. I tried to
   patch it properly and the benchmark are the same whether we use the
   default comparison functor or mine.
  
   So I think you can merge it like that. I have pushed a new version
  without
   the comment.
  
 
  I still haven't tried it, but there are still style issues.
 
 
  * Don't put an else after a return
  * Wrap single line blocks in {}
 
 
  Fixed and force-pushed. Sorry for the inconvenience. I am not used to
 this
  style yet.

 Your Compare::operator() contains this:

  if (j == s2.rend())
{
return false;
}
  return false;

 Any reason not to simplify that?

Because I used to have a comment in the else branch of the if, that I
removed because part of it was wrong
https://github.com/nicolasdespres/CMake/commit/59c871da8b00554812e93ba9c6e47d864424efb0#L0R2023(the
part about the optimization). Then I kept the code without thinking
more about it.


 Also, I don't see why the custom comparison functor is needed at all. I
 removed it and sped up the test significantly. Can you explain?


I had some failing tests because the previous algorithm was not a plain
strcpy().

I have restored the comment. I prefer to keep the too branch even if both
return false to make it clear that the comment apply only in this case and
that it is the only difference with a normal strcpy(3) algorithm.

Cheers,

-- 
Nicolas Desprès
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-29 Thread Stephen Kelly
Nicolas Desprès wrote:

 Also, I don't see why the custom comparison functor is needed at all. I
 removed it and sped up the test significantly. Can you explain?

 
 I had some failing tests because the previous algorithm was not a plain
 strcpy().

I don't see any strcpy(). Anyway, I think that's as much useful review I can 
do for you. I think Brad will have to do the rest.

Thanks,

Steve.



--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-29 Thread Nicolas Desprès
On Mon, Jul 29, 2013 at 2:25 PM, Stephen Kelly steve...@gmail.com wrote:

 Nicolas Desprès wrote:

  Also, I don't see why the custom comparison functor is needed at all. I
  removed it and sped up the test significantly. Can you explain?
 
 
  I had some failing tests because the previous algorithm was not a plain
  strcpy().

 I don't see any strcpy(). Anyway, I think that's as much useful review I
 can
 do for you. I think Brad will have to do the rest.


Arg. Sorry for being unclear (English is not my mother tongue).

The std::lessstring comparator do the same thing (as strcpy()  0).
Unfortunately, we cannot use it here because the previous (O(N))
implementation of cmMakefile::GetSourceFileWithOutput(const char*) was more
complex than that (have a look to the code my path replaces). Of course, I
first tried with the default comparator to save me some work but some tests
were failing. Namely: ExportImport and CustomCommand.

Hope this help.
Thanks for your time.
-Nico
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-26 Thread Nicolas Desprès
On Wed, Jul 24, 2013 at 7:56 PM, Nicolas Desprès
nicolas.desp...@gmail.comwrote:




 On Wed, Jul 24, 2013 at 12:18 PM, Stephen Kelly steve...@gmail.comwrote:

 Nicolas Desprès wrote:

   That's said we can optimize further as I mentioned in my comment (
  
 
 

 https://github.com/nicolasdespres/CMake/commit/59c871da8b00554812e93ba9c6e47d864424efb0#L0R2023
  ).
   Do you have an opinion about it?
 
  Do I understand correctly that the issue is that OutputToSource values
  can be absolute or relative paths? That would be fixable by patching
 only
  UpdateOutputToSourceMap, right?
 
  Not exactly. According to my tests only
  cmMakefiles::GetSourceFileWithOutput's cname argument can be either
  relative or absolute.

 Then patching the callers of GetSourceFileWithOutput is worth looking
 into.
 There are not many. You'd have to try it and see if any issues come up.

 Ok. I will give it a try tomorrow. I hope it won't bring in part of CMake
 I do not know.


Hi Stephen,

I think I need your insight of CMake code base here. According to my tests
(ie ExportImport and CustomCommand), GetSourceFileWithOutput() is called
with a relative path only from here:
https://github.com/Kitware/CMake/blob/master/Source/cmTarget.cxx#L1939

How can I convert this name to an absolute path?

Cheers,

-- 
Nicolas Desprès
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-26 Thread Stephen Kelly
Nicolas Desprès wrote:

 Hi Stephen,
 
 I think I need your insight of CMake code base here. According to my tests
 (ie ExportImport and CustomCommand), GetSourceFileWithOutput() is called
 with a relative path only from here:
 https://github.com/Kitware/CMake/blob/master/Source/cmTarget.cxx#L1939
 
 How can I convert this name to an absolute path?
 

Find out which of the callers of FollowName() call it with a relative path.

Thanks,

Steve.


--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-26 Thread Nicolas Desprès
On Wed, Jul 24, 2013 at 10:48 AM, Nicolas Desprès nicolas.desp...@gmail.com
 wrote:




 On Tue, Jul 23, 2013 at 5:59 PM, Stephen Kelly steve...@gmail.com wrote:

 Nicolas Desprès wrote:

  Hi Stephen,
 
  Did you have any chance to look at the code? I would love to see it
  integrated in the next release. That being said, there is no pressure.
 

 I'll have a look now.

 Thanks!



  +UpdateOutputToSourceMap(outputs, file);

 is missing a 'this-', as per the style. There's a couple of repeats of
 that.

 Done


 Please rename

  typedef std::mapstd::string, cmSourceFile* OutputToInputMap;

 to OutputToSourceMap for similarity to the OutputToSource variable.

 Done


 I haven't tried it out yet, but it looks sane to me. The
 UpdateOutputToSourceMap could probably be faster if outputs is sorted and
 you use lower-bound insertion in a loop over the map or so. If it's fast
 enough already though, probably no need for that :).

 Currently, it is fast enough. The path to optimize was search not the
 insertion. I have not noticed any performance regression in the insertion.
 I liked your idea of inserting in at the end of list/vector and then to
 sort it once the configuration is done but I am afraid this require more
 knowledge of cmake code base than I have and the patch will be harder to
 write.

 That's said we can optimize further as I mentioned in my comment (
 https://github.com/nicolasdespres/CMake/commit/59c871da8b00554812e93ba9c6e47d864424efb0#L0R2023).
 Do you have an opinion about it?

 That was not true!

It was fastest because it was not doing the right thing. I tried to patch
it properly and the benchmark are the same whether we use the default
comparison functor or mine.

So I think you can merge it like that. I have pushed a new version without
the comment.

Thaks,

-- 
Nicolas Desprès
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-24 Thread Nicolas Desprès
On Tue, Jul 23, 2013 at 5:59 PM, Stephen Kelly steve...@gmail.com wrote:

 Nicolas Desprès wrote:

  Hi Stephen,
 
  Did you have any chance to look at the code? I would love to see it
  integrated in the next release. That being said, there is no pressure.
 

 I'll have a look now.

Thanks!



  +UpdateOutputToSourceMap(outputs, file);

 is missing a 'this-', as per the style. There's a couple of repeats of
 that.

Done


 Please rename

  typedef std::mapstd::string, cmSourceFile* OutputToInputMap;

 to OutputToSourceMap for similarity to the OutputToSource variable.

Done


 I haven't tried it out yet, but it looks sane to me. The
 UpdateOutputToSourceMap could probably be faster if outputs is sorted and
 you use lower-bound insertion in a loop over the map or so. If it's fast
 enough already though, probably no need for that :).

Currently, it is fast enough. The path to optimize was search not the
insertion. I have not noticed any performance regression in the insertion.
I liked your idea of inserting in at the end of list/vector and then to
sort it once the configuration is done but I am afraid this require more
knowledge of cmake code base than I have and the patch will be harder to
write.

That's said we can optimize further as I mentioned in my comment (
https://github.com/nicolasdespres/CMake/commit/59c871da8b00554812e93ba9c6e47d864424efb0#L0R2023).
Do you have an opinion about it?


 I'd say the topic can be cleaned up and force pushed for another review.

Done.

-Nico
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-24 Thread Stephen Kelly
Nicolas Desprès wrote:

  +UpdateOutputToSourceMap(outputs, file);

 is missing a 'this-', as per the style. There's a couple of repeats of
 that.

 Done

You seem to have missed this one.

There are also other style issues.

* Don't put an else after a return
* Wrap single line blocks in {} 


 That's said we can optimize further as I mentioned in my comment (
 
https://github.com/nicolasdespres/CMake/commit/59c871da8b00554812e93ba9c6e47d864424efb0#L0R2023).
 Do you have an opinion about it?

Do I understand correctly that the issue is that OutputToSource values can 
be absolute or relative paths? That would be fixable by patching only 
UpdateOutputToSourceMap, right?

Thanks,

Steve.


--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-24 Thread Nicolas Desprès
On Wed, Jul 24, 2013 at 11:29 AM, Stephen Kelly steve...@gmail.com wrote:

 Nicolas Desprès wrote:

   +UpdateOutputToSourceMap(outputs, file);
 
  is missing a 'this-', as per the style. There's a couple of repeats of
  that.
 
  Done

 You seem to have missed this one.

 There are also other style issues.

 * Don't put an else after a return
 * Wrap single line blocks in {}

 Done. I hope I did them all.



  That's said we can optimize further as I mentioned in my comment (
 

 https://github.com/nicolasdespres/CMake/commit/59c871da8b00554812e93ba9c6e47d864424efb0#L0R2023
 ).
  Do you have an opinion about it?

 Do I understand correctly that the issue is that OutputToSource values can
 be absolute or relative paths? That would be fixable by patching only
 UpdateOutputToSourceMap, right?

Not exactly. According to my tests only
cmMakefiles::GetSourceFileWithOutput's cname argument can be either
relative or absolute. When the mapping table is updated it always get an
absolute path. The previous code of GetSourceFileWithOutput() supported
both relative and absolute paths.

Here my test setup:

diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index b68460d..d035e25 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -1029,6 +1029,10 @@ void
 cmMakefile::UpdateOutputToSourceMap(const std::string output,
 cmSourceFile* source)
 {
+  std::cout  UPDATE:   source-GetFullPath()
+  - 
+ output
+ std::endl;
   this-OutputToSource[output] = source;
 }

@@ -2051,6 +2055,7 @@ cmMakefile::Compare::operator()(const std::string s1,
 cmSourceFile *cmMakefile::GetSourceFileWithOutput(const char *cname)
 {
   std::string name = cname;
+  std::cout  QUERY:   name  std::endl;
   OutputToSourceMap::iterator o = this-OutputToSource.find(name);
   if (o != this-OutputToSource.end())
 {

Now when I run either:
ctest -VV -R ExportImport | tee /tmp/ImportExport-master.log
or
ctest -VV -R '^CustomCommand$' | tee /tmp/CustomCommand-master.log

I get some relative path in the QUERY debug message. That's reason why I
wrote the Compare functor because those tests were failing after my first
patch.

I hope it is clearer now.
Cheers,

-Nico
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-24 Thread Stephen Kelly
Nicolas Desprès wrote:

  That's said we can optimize further as I mentioned in my comment (
 

 
https://github.com/nicolasdespres/CMake/commit/59c871da8b00554812e93ba9c6e47d864424efb0#L0R2023
 ).
  Do you have an opinion about it?

 Do I understand correctly that the issue is that OutputToSource values
 can be absolute or relative paths? That would be fixable by patching only
 UpdateOutputToSourceMap, right?

 Not exactly. According to my tests only
 cmMakefiles::GetSourceFileWithOutput's cname argument can be either
 relative or absolute.

Then patching the callers of GetSourceFileWithOutput is worth looking into. 
There are not many. You'd have to try it and see if any issues come up.

Thanks,

Steve.


--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-24 Thread Nicolas Desprès
On Wed, Jul 24, 2013 at 12:18 PM, Stephen Kelly steve...@gmail.com wrote:

 Nicolas Desprès wrote:

   That's said we can optimize further as I mentioned in my comment (
  
 
 

 https://github.com/nicolasdespres/CMake/commit/59c871da8b00554812e93ba9c6e47d864424efb0#L0R2023
  ).
   Do you have an opinion about it?
 
  Do I understand correctly that the issue is that OutputToSource values
  can be absolute or relative paths? That would be fixable by patching
 only
  UpdateOutputToSourceMap, right?
 
  Not exactly. According to my tests only
  cmMakefiles::GetSourceFileWithOutput's cname argument can be either
  relative or absolute.

 Then patching the callers of GetSourceFileWithOutput is worth looking into.
 There are not many. You'd have to try it and see if any issues come up.

 Ok. I will give it a try tomorrow. I hope it won't bring in part of CMake
I do not know.

Cheers,
-Nico
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-23 Thread Nicolas Desprès
Hi Stephen,

Did you have any chance to look at the code? I would love to see it
integrated in the next release. That being said, there is no pressure.

Thanks,
-Nico

On Tue, Jul 16, 2013 at 1:40 PM, Nicolas Desprès
nicolas.desp...@gmail.comwrote:

 Hi Stephen,

 I have pushed more commits and now the problem is fixed and all the test
 suite passes on my machine.

 However it could even be faster. See my comment
 in cmMakefile::Compare::operator():
 https://github.com/nicolasdespres/CMake/blob/7c873c8f3e0b893b1a96d8097105aa79e0477651/Source/cmMakefile.cxx#L2023-L2036

 What do you think?

 Have a good day,
 Nico




 On Mon, Jul 15, 2013 at 5:33 PM, Nicolas Desprès 
 nicolas.desp...@gmail.com wrote:




 On Mon, Jul 15, 2013 at 1:51 PM, Stephen Kelly steve...@gmail.comwrote:

 Nicolas Desprès wrote:

  Yes that one or cmMakefile::GetSourceFileWithOutput(char const*). I
 can
  send you a gzipped of my callgrind data off-list if you want (it
  weights 534K).
 
  If we had a map associating each input to output, we could maybe have
  better performance. WDYT?

 Perhaps. I can probably create the same callgrind data locally anyway,
 but I
 won't have a change until later to dig into this.

 On looking at the cmMakefile::GetSourceFileWithOutput code though, it
 seems
 that it might also be useful to maintain this-SourceFiles
 and (*i)-GetCustomCommand()-GetOutputs() as sorted vectors. They are
 rarely modified, so the O(1) - O(log N) inserts probably won't matter
 much,
 but the O(N) - O(log N) might provide a useful gain. Or, as most access
 is
 at generate time, keep the O(1) insertions, sort() after the configure
 step,
 and then use the binary search at generate time. Of course, I don't know
 if
 there are also configure-time accesses which would be difficult to
 isolate
 from the generate-time ones.

 Does that make any sense?


 I think so.

 I fixed my timeout problem for writing the test, and pushed a first
 draft of a commit fixing this issue.

 I have the remaining test failling:
  56 - ExportImport (Failed)
  87 - CustomCommand (Failed)

 I will investigate them tomorrow.

 --
 Nicolas Desprès




 --
 Nicolas Desprès




-- 
Nicolas Desprès
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-23 Thread Stephen Kelly
Nicolas Desprès wrote:

 Hi Stephen,
 
 Did you have any chance to look at the code? I would love to see it
 integrated in the next release. That being said, there is no pressure.
 

I'll have a look now.


 +UpdateOutputToSourceMap(outputs, file);

is missing a 'this-', as per the style. There's a couple of repeats of 
that.

Please rename 

 typedef std::mapstd::string, cmSourceFile* OutputToInputMap;

to OutputToSourceMap for similarity to the OutputToSource variable.

I haven't tried it out yet, but it looks sane to me. The 
UpdateOutputToSourceMap could probably be faster if outputs is sorted and 
you use lower-bound insertion in a loop over the map or so. If it's fast 
enough already though, probably no need for that :).

I'd say the topic can be cleaned up and force pushed for another review.

Thanks,

Steve.


--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-16 Thread Nicolas Desprès
Hi Stephen,

I have pushed more commits and now the problem is fixed and all the test
suite passes on my machine.

However it could even be faster. See my comment
in cmMakefile::Compare::operator():
https://github.com/nicolasdespres/CMake/blob/7c873c8f3e0b893b1a96d8097105aa79e0477651/Source/cmMakefile.cxx#L2023-L2036

What do you think?

Have a good day,
Nico




On Mon, Jul 15, 2013 at 5:33 PM, Nicolas Desprès
nicolas.desp...@gmail.comwrote:




 On Mon, Jul 15, 2013 at 1:51 PM, Stephen Kelly steve...@gmail.com wrote:

 Nicolas Desprès wrote:

  Yes that one or cmMakefile::GetSourceFileWithOutput(char const*). I can
  send you a gzipped of my callgrind data off-list if you want (it
  weights 534K).
 
  If we had a map associating each input to output, we could maybe have
  better performance. WDYT?

 Perhaps. I can probably create the same callgrind data locally anyway,
 but I
 won't have a change until later to dig into this.

 On looking at the cmMakefile::GetSourceFileWithOutput code though, it
 seems
 that it might also be useful to maintain this-SourceFiles
 and (*i)-GetCustomCommand()-GetOutputs() as sorted vectors. They are
 rarely modified, so the O(1) - O(log N) inserts probably won't matter
 much,
 but the O(N) - O(log N) might provide a useful gain. Or, as most access
 is
 at generate time, keep the O(1) insertions, sort() after the configure
 step,
 and then use the binary search at generate time. Of course, I don't know
 if
 there are also configure-time accesses which would be difficult to isolate
 from the generate-time ones.

 Does that make any sense?


 I think so.

 I fixed my timeout problem for writing the test, and pushed a first
 draft of a commit fixing this issue.

 I have the remaining test failling:
  56 - ExportImport (Failed)
  87 - CustomCommand (Failed)

 I will investigate them tomorrow.

 --
 Nicolas Desprès




-- 
Nicolas Desprès
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-15 Thread Nicolas Desprès
On Sat, Jul 13, 2013 at 10:21 AM, Nicolas Desprès nicolas.desp...@gmail.com
 wrote:




 On Fri, Jul 12, 2013 at 11:35 PM, Stephen Kelly steve...@gmail.comwrote:

 Nicolas Desprès wrote:

  I have pushed my work so far on my github clone of CMake.
  https://github.com/nicolasdespres/CMake/tree/topic/large-deps-perf

 Something must have gone wrong with the push :)

 Oups, sorry :(. Friday evening after a long hard week...


This is now fixed. The patch is  3ac529b4a568dbd78d7ccd9b0ec1443efd75f6ae



 stephen@hal:~/dev/src/cmake{master}$ git show origin/master  | grep
 commit
 commit b9412889e9c5028e32ef3b978d8cdd1175f14b0f
 stephen@hal:~/dev/src/cmake{master}$ git show nicolasdespres/topic/large-
 deps-perf | grep commit
 commit b9412889e9c5028e32ef3b978d8cdd1175f14b0f

 I noticed a lot of obsolete branches in your clone. I noticed a lot of
 branches, but mostly it looks like ninja stuff that's already been merged.
 Is there anything else interesting there?

 Yes I have many old branches I should get rid of. There is only the
 topic/large-deps-perf branch interesting at the moment. I pushed one commit
 on it containing the test I mentioned above. All that work is on my
 computer at the office so I could only fix the push on monday morning.
 Sorry again.

 I also cleaned that up a bit.

Have a good day,

-- 
Nicolas Desprès
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-15 Thread Stephen Kelly
Nicolas Desprès wrote:
  I have pushed my work so far on my github clone of CMake.
  https://github.com/nicolasdespres/CMake/tree/topic/large-deps-perf


Thanks. The bottleneck seems to be in

 cmTargetTraceDependencies::FollowName

as it follows each entry in the LARGE_LIST for each of the 100 targets.


 I also cleaned that up a bit.

At least ninja_convenience_targets seems to be obsolete too? I didn't check 
the others. Just looking through my 'git branch -a' output :).

Thanks,

Steve.


--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-15 Thread Nicolas Desprès
On Mon, Jul 15, 2013 at 11:53 AM, Stephen Kelly steve...@gmail.com wrote:

 Nicolas Desprès wrote:
   I have pushed my work so far on my github clone of CMake.
   https://github.com/nicolasdespres/CMake/tree/topic/large-deps-perf
 

 Thanks. The bottleneck seems to be in

  cmTargetTraceDependencies::FollowName

 as it follows each entry in the LARGE_LIST for each of the 100 targets.


Yes that one or cmMakefile::GetSourceFileWithOutput(char const*). I can
send you a gzipped of my callgrind data off-list if you want (it
weights 534K).


 
  I also cleaned that up a bit.

 At least ninja_convenience_targets seems to be obsolete too? I didn't check
 the others. Just looking through my 'git branch -a' output :).


That's right. I have deleted it.
Thanks,
Nico
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-15 Thread Nicolas Desprès
On Mon, Jul 15, 2013 at 12:45 PM, Nicolas Desprès nicolas.desp...@gmail.com
 wrote:




 On Mon, Jul 15, 2013 at 11:53 AM, Stephen Kelly steve...@gmail.comwrote:

 Nicolas Desprès wrote:
   I have pushed my work so far on my github clone of CMake.
   https://github.com/nicolasdespres/CMake/tree/topic/large-deps-perf
 

 Thanks. The bottleneck seems to be in

  cmTargetTraceDependencies::FollowName

 as it follows each entry in the LARGE_LIST for each of the 100 targets.


 Yes that one or cmMakefile::GetSourceFileWithOutput(char const*). I can
 send you a gzipped of my callgrind data off-list if you want (it
 weights 534K).

If we had a map associating each input to output, we could maybe have
better performance. WDYT?





 
  I also cleaned that up a bit.

 At least ninja_convenience_targets seems to be obsolete too? I didn't
 check
 the others. Just looking through my 'git branch -a' output :).


 That's right. I have deleted it.
 Thanks,
 Nico




-- 
Nicolas Desprès
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-15 Thread Stephen Kelly
Nicolas Desprès wrote:

 Yes that one or cmMakefile::GetSourceFileWithOutput(char const*). I can
 send you a gzipped of my callgrind data off-list if you want (it
 weights 534K).

 If we had a map associating each input to output, we could maybe have
 better performance. WDYT?

Perhaps. I can probably create the same callgrind data locally anyway, but I 
won't have a change until later to dig into this.

On looking at the cmMakefile::GetSourceFileWithOutput code though, it seems 
that it might also be useful to maintain this-SourceFiles 
and (*i)-GetCustomCommand()-GetOutputs() as sorted vectors. They are 
rarely modified, so the O(1) - O(log N) inserts probably won't matter much, 
but the O(N) - O(log N) might provide a useful gain. Or, as most access is 
at generate time, keep the O(1) insertions, sort() after the configure step, 
and then use the binary search at generate time. Of course, I don't know if 
there are also configure-time accesses which would be difficult to isolate 
from the generate-time ones.

Does that make any sense?

Thanks,

Steve.


--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-15 Thread Nicolas Desprès
On Mon, Jul 15, 2013 at 1:51 PM, Stephen Kelly steve...@gmail.com wrote:

 Nicolas Desprès wrote:

  Yes that one or cmMakefile::GetSourceFileWithOutput(char const*). I can
  send you a gzipped of my callgrind data off-list if you want (it
  weights 534K).
 
  If we had a map associating each input to output, we could maybe have
  better performance. WDYT?

 Perhaps. I can probably create the same callgrind data locally anyway, but
 I
 won't have a change until later to dig into this.

 On looking at the cmMakefile::GetSourceFileWithOutput code though, it seems
 that it might also be useful to maintain this-SourceFiles
 and (*i)-GetCustomCommand()-GetOutputs() as sorted vectors. They are
 rarely modified, so the O(1) - O(log N) inserts probably won't matter
 much,
 but the O(N) - O(log N) might provide a useful gain. Or, as most access is
 at generate time, keep the O(1) insertions, sort() after the configure
 step,
 and then use the binary search at generate time. Of course, I don't know if
 there are also configure-time accesses which would be difficult to isolate
 from the generate-time ones.

 Does that make any sense?


I think so.

I fixed my timeout problem for writing the test, and pushed a first draft
of a commit fixing this issue.

I have the remaining test failling:
 56 - ExportImport (Failed)
 87 - CustomCommand (Failed)

I will investigate them tomorrow.

-- 
Nicolas Desprès
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-13 Thread Nicolas Desprès
On Fri, Jul 12, 2013 at 11:35 PM, Stephen Kelly steve...@gmail.com wrote:

 Nicolas Desprès wrote:

  I have pushed my work so far on my github clone of CMake.
  https://github.com/nicolasdespres/CMake/tree/topic/large-deps-perf

 Something must have gone wrong with the push :)

 Oups, sorry :(. Friday evening after a long hard week...


 stephen@hal:~/dev/src/cmake{master}$ git show origin/master  | grep commit
 commit b9412889e9c5028e32ef3b978d8cdd1175f14b0f
 stephen@hal:~/dev/src/cmake{master}$ git show nicolasdespres/topic/large-
 deps-perf | grep commit
 commit b9412889e9c5028e32ef3b978d8cdd1175f14b0f

 I noticed a lot of obsolete branches in your clone. I noticed a lot of
 branches, but mostly it looks like ninja stuff that's already been merged.
 Is there anything else interesting there?

 Yes I have many old branches I should get rid of. There is only the
topic/large-deps-perf branch interesting at the moment. I pushed one commit
on it containing the test I mentioned above. All that work is on my
computer at the office so I could only fix the push on monday morning.
Sorry again.

Cheers,

-- 
Nicolas Desprès
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

[cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-12 Thread Nicolas Desprès
Hi all,

I am using CMake again since a couple of weeks for a project involving the
build of recognition system. This system is made of hundred sub recognition
system that must be all trained with about ~10k files as input.

I ended up writing something like that:
== BEGIN
# A project with hundred targets depending on one command with 10k
dependencies.
cmake_minimum_required(VERSION 2.8)
project(HundredTargetsDepOnOneCommandWithManyDeps)

include(CTest)

# Define LARGE_LIST with 1 input files.
include(large_list.cmake)

foreach(i ${LARGE_LIST})
  file(WRITE ${i} input)
endforeach(i)

add_custom_command(
  OUTPUT bottle_neck
  COMMAND ${CMAKE_COMMAND} -E touch bottle_neck
  DEPENDS ${LARGE_LIST}
  COMMENT Executing command with large number of dependencies...
  )

set(OUTPUTS )
foreach(i RANGE 100)
  set(_output output_${i})
  add_custom_target(${_output}-target ALL
DEPENDS bottle_neck
)
  list(APPEND OUTPUTS ${_output})
endforeach(i)

add_executable(AlwaysTrue AlwaysTrue.c)
== END

Note that the hundred targets have only one dependency. Running CMake on
this project is slow. The configuration is fast but the generation is very
slow. Here some examples:

With Unix Makefiles

$ time make rebuild_cache
Running CMake to regenerate build system...
-- Configuration done
(this is long)
-- Generation done
(this is long)
-- Build files have been written to: ...
make rebuild_cache  120.54s user 6.08s system 97% cpu 2:10.07 total

Memory used is normal (htop indicates 82488 VIRT) and CPU is at 100%.

$ time make
make  129.46s user 3.76s system 99% cpu 2:14.28 total

With Ninja:

$ time ninja -v rebuild_cache
ninja rebuild_cache  98.89s user 0.10s system 99% cpu 1:39.02 total
$ time ninja
ninja  0.01s user 0.04s system 86% cpu 0.055 total

So here my two questions:
1/ Do you think it is possible to improve the performance? (I have not
profile it yet).
2/ Is there a way to set a timeout to ctest so that my test fail. I have
that in Tests/CMakeLists.txt but it does not work:

  set(_test_name HundredTargetsDepOnOneCommandWithManyDeps)
  add_test(${_test_name} ${CMAKE_CTEST_COMMAND}
--build-and-test
${CMake_SOURCE_DIR}/Tests/${_test_name}
${CMake_BINARY_DIR}/Tests/${_test_name}
--build-two-config
${build_generator_args}
--build-project ${_test_name}
--timeout 10
--test-command AlwaysTrue)
  list(APPEND TEST_BUILD_DIRS
${CMake_BINARY_DIR}/Tests/${_test_name})
  unset(_test_name)

I have pushed my work so far on my github clone of CMake.
https://github.com/nicolasdespres/CMake/tree/topic/large-deps-perf

I am willing to do the work on this topic but I wanted to have your insight
first.

Thanks,

-- 
Nicolas Desprès
--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers

Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies

2013-07-12 Thread Stephen Kelly
Nicolas Desprès wrote:

 I have pushed my work so far on my github clone of CMake.
 https://github.com/nicolasdespres/CMake/tree/topic/large-deps-perf

Something must have gone wrong with the push :)

stephen@hal:~/dev/src/cmake{master}$ git show origin/master  | grep commit
commit b9412889e9c5028e32ef3b978d8cdd1175f14b0f
stephen@hal:~/dev/src/cmake{master}$ git show nicolasdespres/topic/large-
deps-perf | grep commit
commit b9412889e9c5028e32ef3b978d8cdd1175f14b0f

I noticed a lot of obsolete branches in your clone. I noticed a lot of 
branches, but mostly it looks like ninja stuff that's already been merged. 
Is there anything else interesting there?

Thanks,

Steve.


--

Powered by www.kitware.com

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers