Re: [cmake-developers] CMake is slow for project with hundred target and one command with large number of dependencies
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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